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

[Sergei Shtylyov <sshtylyov@ru.mvista.com>]

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