Thanks Daniel. A small point. When you send patches could you send them in-line in the mail. It's much easier to deal with and keeps the flow of though going.
On Thu, 2006-08-10 at 22:37 -0400, Daniel Richard G. wrote: > Hello list, > > Finally built and semi-figured-out git. A few more wrinkles ironed out in > this patch. > > > A walkthrough, in order: > > Makefile: > > * Invoke autoheader in the configure target, 'cause it sure as heck hasn't > been run in a while :-) True. > > (You might want to update config.h.in, given that it uses an #ident > directive that gcc complains about in pedantic mode) All the CVS identification will be removed when I get to it. I'll need to put this to one side for a while and merge it by hand as stress testing autofs has exposed a number of more nasty problems that compiler warnings. Just so we're on the same page, what CFLAGS are you using? I'm fairly lazy about compiler warnings most of the time. You wouldn't guess, would you. > > Makefile.conf.in: > > * Added a dummy @substitution@ for datarootdir, so that config.status > doesn't print a warning at the end of a configure script run. OK. I'll have a look at that. > > configure.in: > > * Autoconf (or was it autoheader?) really wants you to use the > three-argument form of AC_DEFINE(). Yep. > > daemon/indirect.c, daemon/lookup.c: > > * Removed random redundant variable declarations. There's more already. > > include/automount.h: > > * From the way it's used, shouldn't the status field be a time_t? This gets > rid of some signed-vs-unsigned comparisons, at the very least. Yes. I do need to spend time on the signed-vs-unsigned issues as they can cause unusual problems. I'll pay special attention to this soonish. > > * Don't use a generic local variable name like "status" in a macro > definition. (This was shadowing local "status" variables in numerous > places.) Did I say lazy! > > include/state.h, lib/alarm.c: > > * Same deal with "status" local variable inside a macro definition. > > lib/parse_subs.c: > > * Got a signed-vs-unsigned comparison warning here. Is it kosher to use > __SWORD_TYPE? I don't know if "struct statfs" is officially supposed to > use more formal type names for its fields. Me neither but I need it so I include it. > > modules/lookup_file.c, modules/parse_hesiod.c: > > * Signed-vs-unsigned comparison fixes. > > modules/lookup_hosts.c: > > * The rpcgen-generated mount.h actually typedefs a "name" type. This makes > the compiler look askance at subsequent, innocuous uses of variables > named "name", so shove aside the typedef with a temporary #define. That's been annoying me for a while now. > > * Fixed more weirdness of using sprintf()'s return value. > > modules/parse_sun.c: > > * The function already has a local variable "ent", of "const char *" type. > I renamed this one to "xent", but you might have a better name for it. Missed that. > > > That takes care of pretty much all the warnings that can be addressed > without annoyingly invasive modifications (assuming you'd want to address > them at all). Here is what remains, with commentary: > > 294 warning: ISO C does not permit named variadic macros > 174 warning: ISO C99 requires rest arguments to be used > > (Well, what can you do?) > > 22 warning: cast discards qualifiers from pointer target type > > (This seems to be done deliberately, so no prob) No sure I what to do about this. I'd like to rationalize it sometime but I always break stuff when I try. There's more important issues to address at the moment. > > 11 warning: ISO C forbids conversion of object pointer to function > pointer type > > (You have no choice when you're using dlsym()!) > > 5 warning: shadowed declaration is here > 5 warning: declaration of ‘_buffer’ shadows a previous local > > (From nested instances of pthread_cleanup_push()) > > 4 warning: comma at end of enumerator list > 1 warning: negative integer implicitly converted to unsigned type > > (rpcgen wackiness) > > 1 warning: comparison between signed and unsigned > > (In Flex-generated code for nss_tok.l) > > > --Daniel > > > _______________________________________________ > autofs mailing list > [email protected] > http://linux.kernel.org/mailman/listinfo/autofs _______________________________________________ autofs mailing list [email protected] http://linux.kernel.org/mailman/listinfo/autofs
