On Fri, Sep 5, 2008 at 11:32 AM, Timothy S. Nelson <wayland@xxxxxxxxxxxxx> wrote: > On Wed, 3 Sep 2008, Michael Kerrisk wrote: > >> Timothy S. Nelson wrote: >>> >>> On Tue, 2 Sep 2008, Michael Kerrisk wrote: >>> >>>> Hello Timothy, >>>> >>>> Timothy S. Nelson wrote: >>>>> >>>>> Updated information obtained from: >>>>> a) Reading the source of search.h on my Fedora system (uses >>>>> __USE_GNU >>>>> instead of _GNU_SOURCE) -- this works in my test program >>>> >>>> This is wrong. See <feature.h> and feature_test_macros(7). >>> >>> Great! Thanks. I'll update the patch so that it tells people to >>> #include <feature.h>. >> >> From feature_test_macros(7): >> >> NOTES >> <features.h> is a Linux/glibc-specific header file. >> Other systems have an analogous file, but typically with >> a different name. This header file is automatically >> included by other header files as required: it is not >> necessary to explicitly include it in order to employ >> feature test macros. >> >>>>> b) Reading the documentation: >>>>> >>>>> http://www.gnu.org/software/libc/manual/html_node/Hash-Search-Function.html#Hash-Search-Function >>>> >>>> >>>> Your mail contains no description of what changes your patch makes, >>>> or why they should be useful. (I am not a mind reader ;-).) >>> >>> I guess I assumed you'd take a skim over the patch to find that out, >>> but I can see why you wouldn't :). >>> >>> Here's a summary of the changes: >>> - _GNU_SOURCE didn't work for me, so I'm updating the documentation so >>> that it will work for others (now #include <feature.h>) >> >> If it didn't work for you then you probably didn't define the FTM before >> including *any* header file. Including <features.h> should never be >> necessary. Please read the preceding paragraph carefully. The same information is provided in the second paragraph of feature_test_macros(7), which I already suggested you read a couple of times. > ------------------------------------------------------------ #define _GNU_SOURCE > #include <stdio.h> > #include <stdlib.h> > > /* #define __USE_GNU */ > #define _GNU_SOURCE > #include <search.h> /* hcreate, etc */ > > int main(void) { > void *memspace; > > memspace = malloc(sizeof(struct hsearch_data)); > > return; > } > ------------------------------------------------------------ > > The code above produces this on my Fedora 6 box: > > hashexample.c: In function 'main': > hashexample.c:11: error: invalid application of 'sizeof' to incomplete type > 'struct hsearch_data' > > > If I change it to __USE_GNU, it works. #including features.h makes > no difference either way. I'm not sure who's to blame for this, but it'd be > nice if the man page made some appropriate comment :). > >>> I can name >>> two advantages: > > [to using subheadings] >>> >>> - It makes it easier to quickly locate information >>> - It immediately highlighted the fact that the "ret" argument was not >>> documented at all (which is something I've corrected in this patch) >> >> Okay -- I already fixed that in my revision of the page. >> (But I got the name wrong: s/retval/ret/) > > Yeah, I saw that later; that's why it didn't show up. I was thinking > more along the lines of general principles, though -- if there are 4 args, > and 4 headings, then something is right :). But if that's not The Man Page > Way(TM), then I'm sure I'll cope :). > >>>> Also, you've removed the >>>> ..SH RETURN VALUE >>>> section, which really should be present in every .2 and .3 page >>>> (though it is currently missing from several). >>> >>> I've divided the Return Value into two separate sections, as you've >>> seen. This is one of the reasons why I was wondering if there shouldn't be >>> at least two separate man pages here; one for hsearch/hsearch_r, and one for >>> everything else. >> >> One could debate the point in either direction (and man-pages >> has examples of doing things both ways). In this case, I think >> the information can be conveyed compactly enough that it makes sense >> to describe everything on one page. > > Ok. I think I would've preferred a longer, more descriptive page, > but then, I'm a Perl programmer :). That idea (of the longer, more > descriptive page, plus probably the Unix "one task, one tool" philosophy) > led me in the direction of more pages with more information. But with the > rewrite you did, plus the reorg below, I'm not too fussed; it's much > improved already :). > >>> If your objection is that it's not marked ".SS", that's probably >>> because I know nothing about groff. Visually (with nroff -man), it somewhat >>> resembles the information on the printf page. >> >> There is no point in citing the strange exceptions. (Many things in >> printf.3 could be tidied.) I'm interested in consistency with >> the general layout in man-pages, also described in man-pages(7). >> I'm not inclined (without very good reason) to accept changes that >> create more divergence from the "standard" layout. > > Sure, no worries :). > >>> In review, it seems to me like I agree with you, except for: >>> - I think re-entrant functions should be discussed with their >>> brethren, >>> rather than being discussed afterwards >> >> Okay -- I'm not averse to that change. I'll revise the page in that >> way and see how it looks. > > Sounds good. Let me know if you want me to review it when you're > done. > > It seems like the only remaining point of discussion is the > _GNU_SOURCE business above. > > HTH, > > > --------------------------------------------------------------------- > | Name: Tim Nelson | Because the Creator is, | > | E-mail: wayland@xxxxxxxxxxxxx | I am | > --------------------------------------------------------------------- > > ----BEGIN GEEK CODE BLOCK---- > Version 3.12 > GCS d+++ s+: a- C++$ U+++$ P+++$ L+++ E- W+ N+ w--- V- PE(+) Y+>++ PGP->+++ > R(+) !tv b++ DI++++ D G+ e++>++++ h! y- > -----END GEEK CODE BLOCK----- > > -- Michael Kerrisk Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/ man-pages online: http://www.kernel.org/doc/man-pages/online_pages.html Found a bug? http://www.kernel.org/doc/man-pages/reporting_bugs.html -- To unsubscribe from this list: send the line "unsubscribe linux-man" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
