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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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-ide" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Filesystems]     [Linux SCSI]     [Linux RAID]     [Git]     [Kernel Newbies]     [Linux Newbie]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Samba]     [Device Mapper]

  Powered by Linux