On Tue, Oct 01, 2013 at 06:49:42PM +0300, Ville Syrjälä wrote:
On Tue, Oct 01, 2013 at 02:06:13PM +0100, Chris Wilson wrote:
CC drivers/gpu/drm/drm_edid_load.o drivers/gpu/drm/drm_edid_load.c: In function ‘drm_load_edid_firmware’: include/linux/err.h:39:17: warning: ‘edid’ may be used uninitialised in this function [-Wuninitialized] drivers/gpu/drm/drm_edid_load.c:141:22: note: ‘edid’ was declared here
In the process, we can make the error handling more resilient.
Signed-off-by: Chris Wilson chris@chris-wilson.co.uk
drivers/gpu/drm/drm_edid_load.c | 75 +++++++++++++++++---------------------- 1 file changed, 32 insertions(+), 43 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid_load.c b/drivers/gpu/drm/drm_edid_load.c index 271b42b..4b57a4c 100644 --- a/drivers/gpu/drm/drm_edid_load.c +++ b/drivers/gpu/drm/drm_edid_load.c @@ -136,59 +136,51 @@ static u8 generic_edid[GENERIC_EDIDS][128] = { static u8 *edid_load(struct drm_connector *connector, const char *name, const char *connector_name) {
- const struct firmware *fw;
- const struct firmware *fw = NULL; struct platform_device *pdev;
- u8 *fwdata = NULL, *edid, *new_edid;
- int fwsize, expected;
- int builtin = 0, err = 0;
- u8 *fwdata, *edid;
Orthogonal issue, but fwdata, generic_edid and generic_edid_names could all be const.
int fwsize, expected, err, builtin; int i, valid_extensions = 0; bool print_bad_edid = !connector->bad_edid_counter || (drm_debug & DRM_UT_KMS);
pdev = platform_device_register_simple(connector_name, -1, NULL, 0);
- if (IS_ERR(pdev)) {
DRM_ERROR("Failed to register EDID firmware platform device ""for connector \"%s\"\n", connector_name);err = -EINVAL;goto out;- }
- err = request_firmware(&fw, name, &pdev->dev);
- platform_device_unregister(pdev);
- if (!IS_ERR(pdev)) {
err = request_firmware(&fw, name, &pdev->dev);platform_device_unregister(pdev);- } else
err = PTR_ERR(pdev);
- if (err) {
- if (err == 0) {
fwdata = (u8 *)fw->data;fwsize = fw->size;builtin = 0;- } else { i = 0; while (i < GENERIC_EDIDS && strcmp(name, generic_edid_name[i])) i++;
if (i < GENERIC_EDIDS) {err = 0;builtin = 1;fwdata = generic_edid[i];fwsize = sizeof(generic_edid[i]);
if (i >= GENERIC_EDIDS) {DRM_ERROR("Requesting EDID firmware \"%s\" failed (err=%d)\n",name, err);edid = ERR_PTR(err);goto out;Due to the 'if (fw)' check in the cleanup code, you could eliminate the out label.
}
}
if (err) {
DRM_ERROR("Requesting EDID firmware \"%s\" failed (err=%d)\n",name, err);goto out;}
if (fwdata == NULL) {
fwdata = (u8 *) fw->data;fwsize = fw->size;
fwdata = generic_edid[i];fwsize = sizeof(generic_edid[i]);builtin = 1;}
expected = (fwdata[0x7e] + 1) * EDID_LENGTH;
Not your bug, but we're missing a check for fwsize > 0x7e.
Can't spot any real bugs, so w/ or w/o the out label idea: Reviewed-by: Ville Syrjälä ville.syrjala@linux.intel.com
Might as well spend the time to fix up the little warts whilst we are here, so expect a v2 shortly. -Chris