On 11/24/2014 02:40 PM, Christoph Hellwig wrote:
> Can someone review this trivial patch for me?  Thanks!
>
The compiler complains because when asd_find_flash_de fails, the offs is not 
initialized.
When that happens this code is invoked :
        if (err) {
                ASD_DPRINTK("couldn't find CTRL-A user settings section\n");
                ASD_DPRINTK("Creating default CTRL-A user settings section\n");

                dflt_ps.id0 = 'h';
                dflt_ps.num_phys = 8;
                for (i =0; i < ASD_MAX_PHYS; i++) {
                        memcpy(dflt_ps.phy_ent[i].sas_addr,
                               asd_ha->hw_prof.sas_addr, SAS_ADDR_SIZE);
                        dflt_ps.phy_ent[i].sas_link_rates = 0x98;
                        dflt_ps.phy_ent[i].flags = 0x0;
                        dflt_ps.phy_ent[i].sata_link_rates = 0x0;
                }

                size = sizeof(struct asd_ctrla_phy_settings);
                ps = &dflt_ps;
the dflt_ps is initialized and the address assigned to 'ps', but none of them 
is used later
 }

        if (size == 0)
                goto out;

        err = -ENOMEM;
        el = kmalloc(size, GFP_KERNEL);
        if (!el) {
                ASD_DPRINTK("no mem for ctrla user settings section\n");
                goto out;
        }

        err = asd_read_flash_seg(asd_ha, (void *)el, offs, size);
        if (err) {
                ASD_DPRINTK("couldn't read ctrla phy settings section\n");
                goto out2;
        }

        err = -ENOENT;
        ps = asd_find_ll_by_id(el, 'h', 0xFF);
here^ a new value is assigned to 'ps'


I have no idea what was intended in the originally so - it looks likely, but 
how do we know that
the '0' is a correct value for offs?

Probably the error path is never used, this patch should fix the gcc warning 
too -
@@ -990,20 +990,7 @@ static int asd_process_ctrl_a_user(struct asd_ha_struct 
*asd_ha,
        err = asd_find_flash_de(flash_dir, FLASH_DE_CTRL_A_USER, &offs, &size);
        if (err) {
                ASD_DPRINTK("couldn't find CTRL-A user settings section\n");
-               ASD_DPRINTK("Creating default CTRL-A user settings section\n");
-
-               dflt_ps.id0 = 'h';
-               dflt_ps.num_phys = 8;
-               for (i =0; i < ASD_MAX_PHYS; i++) {
-                       memcpy(dflt_ps.phy_ent[i].sas_addr,
-                              asd_ha->hw_prof.sas_addr, SAS_ADDR_SIZE);
-                       dflt_ps.phy_ent[i].sas_link_rates = 0x98;
-                       dflt_ps.phy_ent[i].flags = 0x0;
-                       dflt_ps.phy_ent[i].sata_link_rates = 0x0;
-               }
-
-               size = sizeof(struct asd_ctrla_phy_settings);
-               ps = &dflt_ps;
+               goto out;
        }
 
        if (size == 0)
................

Somebody with access to this hw might help and test "offs = 0;".
It's a question whether we should touch old drivers at all.

Tomas



--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to