[Toybox] Toybox Digest, Vol 21, Issue 8

[Kyungwan Han]

Hello all.

I have a basic question.

I have known that "Magic number should not used in the code."
But I checked Rob's review comments and I come to know my though is wrong.

Then, when should I use MACRO instead of magic number?

And close() is not strictly requried in the comments,
but I also know close() MUST be there.
Is my thought wrong?

Thanks.
Kyungwan Han.


2013. 7. 13. 오전 5:37에 <toybox-request at lists.landley.net>님이 작성:
>
> Send Toybox mailing list submissions to
>         toybox at lists.landley.net
>
> To subscribe or unsubscribe via the World Wide Web, visit
>         http://lists.landley.net/listinfo.cgi/toybox-landley.net
> or, via email, send a message with subject or body 'help' to
>         toybox-request at lists.landley.net
>
> You can reach the person managing the list at
>         toybox-owner at lists.landley.net
>
> When replying, please edit your Subject line so it is more specific
> than "Re: Contents of Toybox digest..."
>
>
> Today's Topics:
>
>    1. [CLEANUP] eject commit 947 (Rob Landley)
>
>
> ----------------------------------------------------------------------
>
> Message: 1
> Date: Fri, 12 Jul 2013 00:11:19 -0500
> From: Rob Landley <rob at landley.net>
> To: toybox at lists.landley.net
> Subject: [Toybox] [CLEANUP] eject commit 947
> Message-ID: <1373605879.27613.47 at driftwood>
> Content-Type: text/plain; charset=us-ascii; DelSp=Yes; Format=Flowed
>
> Eject cleanups in commit 947
>
>    http://landley.net/hg/toybox/rev/947
>
> To start with, this isn't just "not in susv4", it isn't in LSB 4.1
> either. So say "no standard".
>
> The usage message: collate the option flags into one [-block]. The
> square brackets mean something is optional, an eplipsis (...) means
> something can be repeated an arbitrary number of times (this is more or
> less copied from man pages), but the device argument can occur zero or
> one times, so [DEVICE] is the way to say that. Use one tab instead of
> spaces between the option and its description. (This is how the others
> do it, saves a couple bytes.)
>
> The #defines are essentially comments: the magic constant is here in
> the C file anyway, it might as well be at the call site with a comment
> about the macro name the constant stands for. Note that DRIVE_NOT_READY
> was never used.
>
>
> --- remove_scsi():
>
> for one line comments I tend to use // instead of /* */ because c99
> allows it. The int argument is a file descriptor so it's standard to
> say "fd" for that. (If you see "fd" you generally know what it is, just
> like i is probably a loop counter.)
>
> I generally have variable declarations as a single block at the start
> of a function, separated from non-declaration statements by a space.
> This had two blocks with a space between them, so collate. While I'm at
> it: we have "toybuf" so declaring structs and arrays on the stack isn't
> really necessary, use chunks of toybuf instead. (The reason I did
> toybuf+64 instead of toybuf+sizeof(blah) is I dunno what the alignment
> of the struct is, or what alignment considerations the buffers have.
> This way everything's 32-byte aligned.
>
> Using toybuf means the memset() can go away: is starts zeroed. I
> renamed in_out_header to just "header" because it's a local variable
> and the smaller the scope the simpler the name you can get away with.
>
> Ripped out the ioctl() querying the version, on the theory A) we don't
> really support old pre-2.0 kernels anyway, B) the failure is to exit
> without ejecting, which is pretty much the failure if we did run on
> such an old kernel somehow. (We wouldn't compile against its libc, but
> oh well.)
>
> --- eject_main():
>
> Move rc declaration into the else {} block that actually uses it, and
> don't bother initializing it to 0 since the only use starts with an
> unconditional assignment.
>
> Move "/dev/cdrom" assignment into variable declaration, and then
> conditionally assign to it if we have an argument. (Not only is this
> one fewer line of code, but the compiler can turn the if () into a
> conditional assignment avoiding a jump and corresponding branch
> prediction, and it's the same amount of assignment code either way so
> the binary isn't any larger.) Put a space between variable declarations
> and non-declaration code. And have the test the same thing we're
> assigning instead of a different variable so we load into a register
> and then re-use that register. (The command line argument array is null
> terminated so toys.optargs[toys.optc] is guaranteed to be NULL the same
> way argv[argc] is guaranteed to be NULL. I don't remember if that's the
> C standard, posix, or the ELF spec, but I tracked down where it was
> standardized once and it is, not just "happens to be true" but
> required.)
>
> The above #defines substitued in as constants start kicking in here,
> not the comments at end of line. The if (TRAY_OPEN) ioctl(EJECT) stuff
> could be on one line, but they're on two lines so each line can have a
> trailing comment with the symbol name that should theoretically be in
> some header somewhere, but isn't.
>
> Inserted extra parentheses:
>
> -    if (toys.optflags & FLAG_T || toys.optflags & FLAG_t) {
> +    if ((toys.optflags & FLAG_T) || (toys.optflags & FLAG_t)) {
>
> -      if (toys.optflags & FLAG_t || rc == 2) //CDS_TRAY_OPEN = 2
> +      if ((toys.optflags & FLAG_t) || rc == 2)         // CDS_TRAY_OPEN
>
> Mixing logical and boolean operators, the precedence is nonobvious.
> Dennis Ritchie wrote about the historical reasons in his History of
> Programming Languages conference talk:
>
>    http://cm.bell-labs.com/cm/cs/who/dmr/chist.html
>
> Relevant chunk:
>
> > At the suggestion of Alan Snyder, I introduced the && and || operators
> > to make the mechanism more explicit. Their tardy introduction explains
> > an infelicity of C's precedence rules. In B one writes
> >
> >   if (a==b & c) ...
> >
> > to check whether a equals b and c is non-zero; in such a conditional
> > expression it is better that & have lower precedence than ==. In
> > converting from B to C, one wants to replace & by && in such a
> > statement;
> > to make the conversion less painful, we decided to keep the precedence
> > of the & operator the same relative to ==, and merely split the
> > precedence of && slightly from &. Today, it seems that it would have
> > been preferable to move the relative precedences of & and ==, and
> > thereby
> > simplify a common C idiom: to test a masked value against another
> > value,
> > one must write
> >
> >   if ((a&mask) == b) ...
> >
> > where the inner parentheses are required but easily forgotten.
>
> And this means whenever I have a boolean operation mixed with logical
> operators, I parenthesize even when it's not strictly required.
>
> The trailing comments are off to the right a bit because there's
> several of them so lining them up helps readability.
>
> The close isn't strictly required, so I put it in CFG_TOYBOX_FREE which
> is basically the "I'm going to run this under valgrind and want to
> reduce the noise" config symbol.
>
> (And xclose only actually matters on NFS, no other filesystem can
> return an error from close() that I know of, but as long as it's
> there...)
>
> Rob
>
> ------------------------------
>
> _______________________________________________
> Toybox mailing list
> Toybox at lists.landley.net
> http://lists.landley.net/listinfo.cgi/toybox-landley.net
>
>
> End of Toybox Digest, Vol 21, Issue 8
> *************************************
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.landley.net/pipermail/toybox-landley.net/attachments/20130717/1e2aa557/attachment-0004.htm>