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
if (expected != fwsize) { DRM_ERROR("Size of EDID firmware "%s" is invalid " "(expected %d, got %d)\n", name, expected, (int) fwsize);
err = -EINVAL;
edid = ERR_PTR(-EINVAL);goto relfw_out; }
edid = kmemdup(fwdata, fwsize, GFP_KERNEL); if (edid == NULL) {
err = -ENOMEM;
edid = ERR_PTR(-ENOMEM);@@ -197,7 +189,7 @@ static u8 *edid_load(struct drm_connector *connector, const char *name, DRM_ERROR("Base block of EDID firmware "%s" is invalid ", name); kfree(edid);
err = -EINVAL;
edid = ERR_PTR(-EINVAL);@@ -210,19 +202,18 @@ static u8 *edid_load(struct drm_connector *connector, const char *name, }
if (valid_extensions != edid[0x7e]) {
u8 *new_edid;- edid[EDID_LENGTH-1] += edid[0x7e] - valid_extensions; DRM_INFO("Found %d valid extensions instead of %d in EDID data " ""%s" for connector "%s"\n", valid_extensions, edid[0x7e], name, connector_name); edid[0x7e] = valid_extensions;
- new_edid = krealloc(edid, (valid_extensions + 1) * EDID_LENGTH,
GFP_KERNEL);if (new_edid == NULL) {err = -ENOMEM;kfree(edid);goto relfw_out;}edid = new_edid;
GFP_KERNEL);if (new_edid)edid = new_edid;}
DRM_INFO("Got %s EDID base block and %d extension%s from "
@@ -231,12 +222,10 @@ static u8 *edid_load(struct drm_connector *connector, const char *name, name, connector_name);
relfw_out:
- release_firmware(fw);
- if (fw)
release_firmware(fw);out:
- if (err)
return ERR_PTR(err);- return edid;
}
-- 1.7.9.5
dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel