Re: [PATCH] aic94xx: Fixup compilation warning

[Tomas Henzl <thenzl@redhat.com>]

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