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