On Sun, Feb 02, 2003 at 05:00:08PM -0800, Marty Biznatch wrote:
> In the cdrom_check_status routine there is the code:
>
> static int cdrom_check_status(ide_drive_t *drive,
> struct request_sense *sense)
> 2063 {
> 2064 struct request req;
> 2065 struct cdrom_info *info =
> drive->driver_data;
> 2066 struct cdrom_device_info *cdi =
> &info->devinfo;
> 2067
> 2068 cdrom_prepare_request(&req);
> 2069
> 2070 req.sense = sense;
> 2071 req.cmd[0] = GPCMD_TEST_UNIT_READY;
> 2072
> 2073 #if ! STANDARD_ATAPI
> 2074 /* the Sanyo 3 CD changer uses byte 7 of
> TEST_UNIT_READY to
> 2075 switch CDs instead of supporting the
> LOAD_UNLOAD opcode */
> 2076
> 2077 req.cmd[7] = cdi->sanyo_slot % 3;
> 2078 #endif /* not STANDARD_ATAPI */
> 2079
> 2080 return cdrom_queue_packet_command(drive,
> &req);
> 2081 }
>
> Would this code be more readable and perhaps more
> corectly written if the structures cdrom_info and
> cdrom_device_info were conditionaly assembled as the
> #if ! STANDARD_ATAPI line is?
> The optimizer will not allocate space for these
> structures if unused, however it is more code that
> needs to be greped through to understand a routine
> that otherwise could be ignored? The negative of
> course being that conditional ifs are ugly.
Memory space is not an issue, code readability is. You'd rather do
something like this in the header file:
#ifndef STANDARD_ATAPI
#define sanyo_fixup() \
do { \
req.cmd[7] = cdi->sanyo_slot % 3; \
} while(0)
#else
#define sanyo_fixup() do { } while(0);
#endif
So the code can become:
sanyo_fixup();
Which gcc will nicely optimise away and is much more readable.
Erik
--
J.A.K. (Erik) Mouw
Email: J.A.K.Mouw@its.tudelft.nl mouw@nl.linux.org
WWW: http://www-ict.its.tudelft.nl/~erik/
Attachment:
pgp00283.pgp
Description: PGP signature
