Messages in this thread | | | Date | Mon, 22 Sep 2008 19:50:54 +0400 | From | Sergei Shtylyov <> | Subject | Re: [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
| |