Hi, 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.) > > > + 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 :-) > > Also, I wonder whether the macro would break if you do: > > > > opt_name = OPT_LONG(transparent_mount ? OPT_LONG_MOUNT : > > OPT_LONG_NOMOUNT) > > I had tried this before posting the patch; it breaks. > > Take a look at the definition of OPT_LONG: > > #define OPT_LONG(o) "--" o I see. -antrik-
