Designing fgetline - a perspective
in [General Programming]
|
Prev: problem analysis chart
Next: Please help!
From: Ben Bacarisse on 9 Nov 2007 12:00 cri(a)tiac.net (Richard Harter) writes: > On Wed, 7 Nov 2007 11:08:30 +0100, "Charlie Gordon" > <news(a)chqrlie.org> wrote: > >>"Richard Harter" <cri(a)tiac.net> a écrit dans le message de news: >>4729fd77.329292093(a)news.sbtc.net... >>> >><snipped long interesting discussion> >>> >>> Finally, what should the interface look like? I now believe that >>> the prototype should look like this: >>> >>> int getfline(FILE * fptr, >>> char ** line_ptr, >>> long * status, >>> size_t * length_ptr, >>> size_t maxlen); >>> <snip> >>- the return value could be more informative: we could make it an ssize_t to >>return the length of the line read or EOF upon end of file. It would also >>be more intuitive as a replacement for fgets() if it returned a pointer to >>the char buffer. <snip> > Thanks muchly for the comments. I think I will follow most of > your suggestions. AFAICT bit fields would be the right way > to go. Using a struct to hold most of the stuff might be > cleaner, though I'm still waffling on that one. Returning a > pointer to the line is probably right. Ugh, I hate rewriting. I would not advocate returning a pointer to a buffer that also "returned" via a char **. The reason is that you get an alias that may subsequently become invalid (or change). Of course, it is not "wrong", but I would be wary of adding that pitfall to your otherwise very clear and flexible API. -- Ben.
From: Richard Harter on 9 Nov 2007 16:13 On Fri, 09 Nov 2007 17:00:57 +0000, Ben Bacarisse <ben.usenet(a)bsb.me.uk> wrote: >cri(a)tiac.net (Richard Harter) writes: > >> On Wed, 7 Nov 2007 11:08:30 +0100, "Charlie Gordon" >> <news(a)chqrlie.org> wrote: [snip] >> Thanks muchly for the comments. I think I will follow most of >> your suggestions. AFAICT bit fields would be the right way >> to go. Using a struct to hold most of the stuff might be >> cleaner, though I'm still waffling on that one. Returning a >> pointer to the line is probably right. Ugh, I hate rewriting. > >I would not advocate returning a pointer to a buffer that also >"returned" via a char **. The reason is that you get an alias that >may subsequently become invalid (or change). Of course, it is not >"wrong", but I would be wary of adding that pitfall to your otherwise >very clear and flexible API. Point well taken. Upon reflection, I'm not sure that bit fields work well in this context. Is there a portable way to set several bits in a bit field struct? Richard Harter, cri(a)tiac.net http://home.tiac.net/~cri, http://www.varinoma.com In the fields of Hell where the grass grows high Are the graves of dreams allowed to die
From: Richard Harter on 9 Nov 2007 17:00 On Fri, 09 Nov 2007 11:36:18 -0500, Eric Sosman <Eric.Sosman(a)sun.com> wrote: >Richard Harter wrote On 11/09/07 10:34,: >> Below is a "man page" for a file read utility. An HTML version >> can be found at http://home.tiac.net/~cri/2007/gflspec.html. It >> is not guaranteed to be typo or brain fart clean. >> >> ---- >> >> SYNOPSIS: >> >> #include "getfline.h" >> >> int getfline(FILE * fptr, >> char ** line_ptr, >> size_t * length_ptr, >> size_t * bufsize_ptr, >> size_t maxlen, >> long * flags); >> [...] > > Isn't there some way you can find an excuse to add >a couple more arguments? Six is too many for most people >to keep straight, but you may as well try to confuse the >geniuses, too. ;-) I take it you didn't check out the list of flags. I'm after the geniuses too. :-) > > Kidding aside, an argument list of that length suggests >to me that the function may be trying to be too many things >to too many people at the same time. It might be wise to >give up some functionality to improve ease of use; the net >change in usefulness could in fact be positive. Point well taken, though there really is nothing that I would want to give up. I wouldn't want to give up error information, nor a bound on maximum line size, nor a reusable buffer (for which both address and size are needed), and not even the line length. One thing that could be done is package some of the arguments in a struct. Frex: struct getfline_args { size_t bufsize, size_t length, size_t maxlen, long flags); Then we can have a prototype that looks like this: int * getfline(FILE *fptr, char **line_ptr, struct getfline_args *args); If we adopt the convention a maxlen of zero means no upper bound check or alternately, another input flag needed to activate bounds checking, then instead of a single long calling sequence we can do: struct getfline_args gfl_args = {0,0,0,0}; char *line = 0; FILE *fptr = 0; ... while (getfline(fptr,&line,&gfl_args) { /* Do stuff */ } This doesn't really change anything but it makes it easier to use the defaults and it moves some of the definitions into the include file. What do you think of this? Richard Harter, cri(a)tiac.net http://home.tiac.net/~cri, http://www.varinoma.com In the fields of Hell where the grass grows high Are the graves of dreams allowed to die
From: Eric Sosman on 9 Nov 2007 18:30 Richard Harter wrote On 11/09/07 17:00,: > On Fri, 09 Nov 2007 11:36:18 -0500, Eric Sosman > <Eric.Sosman(a)sun.com> wrote: > > >>Richard Harter wrote On 11/09/07 10:34,: >> >>>Below is a "man page" for a file read utility. An HTML version >>>can be found at http://home.tiac.net/~cri/2007/gflspec.html. It >>>is not guaranteed to be typo or brain fart clean. >>> >>>---- >>> >>>SYNOPSIS: >>> >>>#include "getfline.h" >>> >>>int getfline(FILE * fptr, >>> char ** line_ptr, >>> size_t * length_ptr, >>> size_t * bufsize_ptr, >>> size_t maxlen, >>> long * flags); >>>[...] >> >> Isn't there some way you can find an excuse to add >>a couple more arguments? Six is too many for most people >>to keep straight, but you may as well try to confuse the >>geniuses, too. ;-) > > > I take it you didn't check out the list of flags. I'm > after the geniuses too. :-) Well, I noticed you needed a `long' to hold them, not a mere `int' ... Close to two dozen flags, aren't there? >> Kidding aside, an argument list of that length suggests >>to me that the function may be trying to be too many things >>to too many people at the same time. It might be wise to >>give up some functionality to improve ease of use; the net >>change in usefulness could in fact be positive. > > Point well taken, though there really is nothing that I would > want to give up. I wouldn't want to give up error information, > nor a bound on maximum line size, nor a reusable buffer (for > which both address and size are needed), and not even the line > length. One thing that could be done is package some of the > arguments in a struct. Frex: > > struct getfline_args { > size_t bufsize, > size_t length, > size_t maxlen, > long flags); > > Then we can have a prototype that looks like this: > > int * getfline(FILE *fptr, > char **line_ptr, > struct getfline_args *args); I'm not sure why the return value changed from an int to a pointer. Typo? > If we adopt the convention a maxlen of zero means no upper bound > check or alternately, another input flag needed to activate > bounds checking, then instead of a single long calling sequence > we can do: > > struct getfline_args gfl_args = {0,0,0,0}; > char *line = 0; > FILE *fptr = 0; > ... > while (getfline(fptr,&line,&gfl_args) { > /* Do stuff */ > } > > This doesn't really change anything but it makes it easier to use > the defaults and it moves some of the definitions into the > include file. What do you think of this? It reawakens memories of the days when interfaces used "control blocks," often populated by assembly macros. The C library's struct tm is such a beast, FILE can be thought of in that light, and POSIX has no shortage of structured arguments. If you take this route, you'll at least be on a well-marked trail. If you like the control block approach, though, why not go whole hog and put the line_ptr in the struct, too? You *could* even park the fptr there, but it could ease things a little if a struct initialized to all zeroes meant something sensible. With a purpose-built struct type handy by, that overwhelming mass of flags might become a bunch of bit-fields. Bit-fields are, IMHO, a mixed blessing, but in a case like this they'd avoid the need to pollute the name space with all those GFL_xxx macros. Still, I can't shake the feeling that you're trying to get one function to do too many tasks. Instead of using a bazillion flags to govern different modes of operation, might you consider a suite of related functions to handle the important variations? In effect, you'd be moving a few flags out of the struct and into the function name. Observe the exec*() family of POSIX interfaces, for example: they all do "the same thing" and they probably all devolve on the same internal implementation, but using different names for different (although related) operations is a useful simplification. Try to imagine what it would be like if all of printf(), fprintf(), sprintf(), vprintf(), vfprintf(), vsprintf(), and vsnprintf() were packaged into one function name with a control block to choose different operation modes ... Ugh! As you may gather, I am a big fan of small, simple interfaces. Some people feel my adoration of simplicity is unhealthily intense; my own line-getter has been criticized for violating the "... but not simpler" part of Einstein's dictum. (The critic was someone I think highly of, so I gave his criticism careful thought before deciding to ignore it.) I mention all this to make my biases clear: the KISS principle is very important to me, but may not hold quite so much sway over others. -- Eric.Sosman(a)sun.com
From: CBFalconer on 9 Nov 2007 19:08
Eric Sosman wrote: > .... snip ... > > As you may gather, I am a big fan of small, simple interfaces. > Some people feel my adoration of simplicity is unhealthily > intense; my own line-getter has been criticized for violating > the "... but not simpler" part of Einstein's dictum. (The > critic was someone I think highly of, so I gave his criticism > careful thought before deciding to ignore it.) I mention all > this to make my biases clear: the KISS principle is very > important to me, but may not hold quite so much sway over others. Here is the heart of my ggets function, which more or less adheres to your principles. The entire package, with testing code, demos, docs, etc. is available at: <http://cbfalconer.home.att.net/download/> #include <stdio.h> #include <stdlib.h> #include "ggets.h" #define INITSIZE 112 /* power of 2 minus 16, helps malloc */ #define DELTASIZE (INITSIZE + 16) enum {OK = 0, NOMEM}; int fggets(char* *ln, FILE *f) { int cursize, ch, ix; char *buffer, *temp; *ln = NULL; /* default */ if (NULL == (buffer = malloc(INITSIZE))) return NOMEM; cursize = INITSIZE; ix = 0; while ((EOF != (ch = getc(f))) && ('\n' != ch)) { if (ix >= (cursize - 1)) { /* extend buffer */ cursize += DELTASIZE; if (NULL == (temp = realloc(buffer, (size_t)cursize))) { /* ran out of memory, return partial line */ buffer[ix] = '\0'; *ln = buffer; return NOMEM; } buffer = temp; } buffer[ix++] = ch; } if ((EOF == ch) && (0 == ix)) { free(buffer); return EOF; } buffer[ix] = '\0'; if (NULL == (temp = realloc(buffer, (size_t)ix + 1))) { *ln = buffer; /* without reducing it */ } else *ln = temp; return OK; } /* fggets */ -- Chuck F (cbfalconer at maineline dot net) <http://cbfalconer.home.att.net> Try the download section. -- Posted via a free Usenet account from http://www.teranews.com |