Re: [PATCH 06/22] ide-tape: struct idetape_tape_t: remove unusedmembers

[Sergei Shtylyov]

Boris Petkov wrote:

> > > @@ -3438,9 +3419,9 @@ static int idetape_identify_device (ide_drive_t
> > > *drive)
> > >  static void idetape_get_inquiry_results(ide_drive_t *drive)
> > > {
> > > -       char *r;
> > >       idetape_tape_t *tape = drive->driver_data;
> > >       idetape_pc_t pc;
> > > +       char fw_rev[6], vendor_id[10], product_id[18];
> > >         idetape_create_inquiry_cmd(&pc);
> > >       if (idetape_queue_pc_tail(drive, &pc)) {
> > > @@ -3448,20 +3429,16 @@ static void
> > > idetape_get_inquiry_results(ide_drive_t *drive)
> > >                               tape->name);
> > >               return;
> > >       }
> > > -       memcpy(tape->vendor_id, &pc.buffer[8], 8);
> > > -       memcpy(tape->product_id, &pc.buffer[16], 16);
> > > -       memcpy(tape->firmware_revision, &pc.buffer[32], 4);
> > > -
> > > -       ide_fixstring(tape->vendor_id, 10, 0);
> > > -       ide_fixstring(tape->product_id, 18, 0);
> > > -       ide_fixstring(tape->firmware_revision, 6, 0);
> > > -       r = tape->firmware_revision;
> > > -       if (*(r + 1) == '.')
> > > -               tape->firmware_revision_num = (*r - '0') * 100 +
> > > -                       (*(r + 2) - '0') * 10 + *(r + 3) - '0';
> > > +       memcpy(vendor_id, &pc.buffer[8], 8);
> > > +       memcpy(product_id, &pc.buffer[16], 16);
> > > +       memcpy(fw_rev, &pc.buffer[32], 4);
> > > +
> > > +       ide_fixstring(vendor_id, 10, 0);
> > > +       ide_fixstring(product_id, 18, 0);
> > > +       ide_fixstring(fw_rev, 6, 0);

> >  It was wrong to call ide_fixstring() on unterminated strings and expecting
> > them to become terminated strings after that; plus it was useless to add 2
> > characters padding at the end. When these variables were the fields of
> > 'struct ide_tape_obj', those bytes were 0 because of the variable of this
> > type being a static array. When they became local variables, they got
> > garbage bytes at the end which ide_fixdriveid() either honestly copied when
> > compressing spaces or just left where they were...

> LOL, i just sent out a similar fix :).

> > > +
> > >       printk(KERN_INFO "ide-tape: %s <-> %s: %s %s rev %s\n",

> >  Should've rather changed the string format to print only N characters
> > max...

   The fix that I have suggested isn't at all similar.  We don't need to 
waste memory on extra bytes, and even less on them being of the 'static' 
memory class...

MBR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/