Hello, On Sat, Aug 08, 2009 at 04:53:29AM +0200, olafbuddenha...@gmx.net wrote: > On Mon, Aug 03, 2009 at 10:39:51PM +0300, Sergiu Ivanov wrote: > > On Wed, Jul 29, 2009 at 09:27:10AM +0200, olafbuddenha...@gmx.net wrote: > > > On Fri, Jul 17, 2009 at 01:56:57PM +0300, Sergiu Ivanov wrote: > > > > > +/* Shows the mode in which the current instance of unionmount > > > > + operates (transparent/non-transparent). */ > > > > +int transparent_mount = 1; > > > > > > I think it would be clearer to default to "0" and set it on --mount... > > > > > > But that's not terribly important really :-) > > > > I set it to 1 because in this case argp_parse_common_options requires > > only the addition of the lines for handling the OPT_NOMOUNT option, > > which resumes to assigning 0 to transparent_mount. Setting this > > variable to 0 initially will require adding one more line and several > > line shuffles. > > Not at all. Unless I'm missing something crucial, you can use exactly > the same method -- only doing =1 in the OPT_MOUNT case instead of =0 for > OPT_NOMOUNT. > > (Of course, you also have to switch the cases, as it's the other one > that needs a fallthrough when doing it this way around.)
This is what I meant when I was talking about ``line shuffling'' :-) > > > > + char * opt_name = (transparent_mount ? OPT_LONG (OPT_LONG_MOUNT) > > > > + : OPT_LONG (OPT_LONG_NOMOUNT)); > > > > > > Don't mix declarations and statements. While C99 allows this, and gcc > > > supported it even before, it's not very good coding style IMHO. I > > > haven't seen it in other Hurd code. > > > > > > (There is a single instance of "for (int i=..." in rpctrace; but even > > > that is questionable -- and it's not quite the same thing anyways...) > > > > OK, thank you for explanation. I had never used this style before and > > wanted to ``try'' it out. > > > > > I don't remember whether GCS says something on that? > > > > I skimmed the ``Making the Best Use of C'' section and didn't notice > > anything (though a more attentive perusal might reveal something). > > Anyways, I've never seen variables initialized at declaration in the > > Hurd, so I won't do like that. > > That's not what I'm talking about here. The GCS *does* say not to > initialize variables at declaration time (or at least it did a couple of > years ago) -- but some existing Hurd code actually ignores this > recommendation, so I don't really consider it a problem. > > What I'm talking about here is that you have a declaration in the middle > of a code block, between statements. This is not allowed in C++ and in > C99, but not in traditional C, where all declarations have to be at the > beginning of a block. > > So moving the whole thing up would be sufficient to address the issue I > was actually talking about -- but splitting it up like you did in the > updated patch is also fine of course :-) Fine :-) I'll stick with splitting declaration and initialization, since it (usually) makes the code more readable (by splitting it into something like ``segments''). Regards, scolobb