lkml.org 
[lkml]   [2008]   [Sep]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 06/22] ide-tape: struct idetape_tape_t: remove unused members
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


\
 
 \ /
  Last update: 2008-09-22 17:53    [W:0.051 / U:0.252 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site