From: James B. <bi...@cs...> - 2005-11-10 21:40:39
|
> I think a stuffN_size_t() type function could be useful in this context. Are you proposing a variable argument function stuffN_size_t() function, or a set of stuff{1,2,3,..9}_size_t() functions? > However, I was only proposing a function name rename, not the removal > of any functions. > > Specifically, renaming: > > function(arg, ...) --> function_va(arg, ...) > function_nva(arg, ptr) --> function(arg, ptr) > > The potential problem is that existing code (outside of Teem) which > uses the old var-args-using "function" may still link fine with the new > non-var-args-using "function", and experience problems (likely cryptic) > only at run time. > > One idea from Dave (via phone) is to do the rename in two stages, to > force code that should change to not compile. First: > > function(arg, ...) --> function_va(arg, ...) > (function_nva(arg, ptr) stays as is) > > And then, in a follow-up release: > > (function_va(arg, ...) stays as is) > function_nva(arg, ptr) --> function() > > This has the benefit that we may decide to just leave things as they > are after the first change, so that its always clear that the functions > come in both var-args and non-var-args form, and you can pick whichever > you're comfortable with. > > Feedback? This seems the most sensible thing so far. You aren't switching types of functions, but rather making one more explicit as to its nature (va == variable arguments). This way you know what you are getting yourself into. Speaking of getting yourself into something with variable arguments, you never responded with the nature of the weirdness when dealing with unsigned int to size_t conversions through the variable argument transformation. James > On Nov 9, 2005, at 1:23 PM, James Bigler wrote: > >>> This message concerns a possible API change related to variable >>> argument lists in nrrd. Please give me your feedback. >>> nrrdMaybeAlloc(nout=nrrdNew(), otype, 4, >>> ansLen, sox, soy, soz); >>> The benefit of the var-arg functions is that you don't have to >>> create a separate array variable to hold the arguments that are >>> passed individually. Gordon also ceded that some users might not >>> like var- args, so all nrrd functions that use var-args have a non- >>> var-args counterpart (whose name always ends with "_nva", for no >>> var-args): >>> nrrdMaybeAlloc_nva(Nrrd *nrrd, int type, unsigned int dim, >>> const size_t *size); >>> which might be used as: >>> size_t size[NRRD_DIM_MAX]; >>> ... >>> size[0] = ansLen; >>> size[1] = sox; >>> size[2] = soy; >>> size[3] = soz; >>> nrrdMaybeAlloc_nva(nout=nrrdNew(), otype, 4, size); >> >> >> For what it's worth, I find the variable argument lists extremely >> useful, in that I don't have to pack everything into an array when I >> know what dimensionality I'm working with. >> >> Perhaps we could add some ugly functions: >> >> size_t* stuff2_size_t(size_t* data, size_t v1, size_t v2) { >> data[0] = v1; >> data[1] = v2; >> return data; >> } >> >> size_t* stuff3_size_t(size_t* data, size_t v1, size_t v2, size_t v3) { >> data[0] = v1; >> data[1] = v2; >> data[2] = v3; >> return data; >> } >> >> ... >> >> Then you could do something like: >> >> size_t sizes[NRRD_DIM_MAX]; >> >> nrrdMaybeAlloc(nrrd, nrrdTypeDouble, 3, stuff3_size_t(sizes, 2, 3, 4)); >> >> You would have to provide your own local storage and pass it in. >> Plus you would have to create an army of little wrapping functions >> for each type of arg, plus the number of args. >> >>> These days, however, Gordon has taken a liking to another >>> programming language concept with a little more gravitas than >>> variable argument lists: type safety. And with that, his love of >>> var-args has been eliminated, because variable arguments in C (and >>> C++) are fundamentally incompatible with any notion of type >>> safety. In fact, the same nrrdMaybeAlloc() example above has been >>> replaced in Teem with: >>> nrrdMaybeAlloc(nout=nrrdNew(), otype, 4, >>> (size_t)ansLen, >>> (size_t)sox, (size_t)soy, (size_t)soz); >>> because on at least one 64-linux system, without the "(size_t)" >>> casts, the values were not correctly received by nrrdMaybeAlloc, >>> and wacky memory errors ensued. This is because the "unsigned >>> int" variables ansLen, sox, etc were 32-bit values, but were >>> retrieved on the nrrdMaybeAlloc side as 64-bit values. What's >>> interesting is that other 64-bit systems don't always choke on >>> this, as if they always use 64-bits for each var-arg parameter, >>> regardless of actual type size. Anyway. >> >> >> How were the 32 bit values being munged up by becoming 64 bit? Can >> you explain why this caused errors or the nature of the error? >> >>> THE PROPOSAL ********************* >>> Therefore, I'm thinking of an API change. From Teem 1.8 to Teem >>> 1.9 there has been one big and proud API change associated with >>> the uniform use of unsigned integral types where appropriate, and >>> the change to "size_t" for all axis size-related info. If the >>> following var-args-related API change makes sense, it would be nice >>> to also put it into Teem 1.9 before its released, to maximize the >>> chance that from Teem 1.9 to 2.0, people will not have to re-write >>> code again. >>> The change: Deprecate the var-arg functions, by renaming them to >>> end with "_va", and removing the "_nva" suffix from the non-var- >>> args- using functions. The idea is that the function name itself >>> is advertising/warning about the use of variable arguments, and >>> the associated absence of type safety that that implies. >>> The worst offenders: >>> NRRD_EXPORT int nrrdWrap(Nrrd *nrrd, void *data, int type, unsigned >>> int dim, >>> ... /* sx, sy, .., axis(dim-1) size */); >>> NRRD_EXPORT int nrrdAlloc(Nrrd *nrrd, int type, unsigned int dim, >>> ... /* sx, sy, .., axis(dim-1) size */); >>> NRRD_EXPORT int nrrdMaybeAlloc(Nrrd *nrrd, int type, unsigned int dim, >>> ... /* sx, sy, .., axis(dim-1) size */); >>> NRRD_EXPORT int nrrdReshape(Nrrd *nout, const Nrrd *nin, unsigned >>> int dim, >>> ... /* sx, sy, .., axis(dim-1) size */ ); >>> NRRD_EXPORT int nrrdSample(void *val, const Nrrd *nin, >>> ... /* coord0, coord1, .., coord (dim-1) >>> */ ); >>> These are also offenders, but the old-fashioned type-promotion >>> rules (read K+R if interested) meant that things looking like >>> doubles probably were promoted to doubles, which reduced the >>> chances that the function incorrectly received the parameters: >>> NRRD_EXPORT int nrrdPad(Nrrd *nout, const Nrrd *nin, >>> const ptrdiff_t *min, const ptrdiff_t *max, >>> int boundary, ... /* if nrrdBoundaryPad, >>> the value */); >>> NRRD_EXPORT int nrrdSimplePad(Nrrd *nout, const Nrrd *nin, unsigned >>> int pad, >>> int boundary, >>> ... /* if nrrdBoundaryPad, what value >>> */); >>> These actually present a different problem, because there are >>> already _nva versions, which are also type-safety disasters (by the >>> use of void*). But that's really a separate issue for a different >>> late- night teem-users post, and the solution is to start creating >>> named accessor functions for each of the axis info fields: >>> NRRD_EXPORT void nrrdAxisInfoSet(Nrrd *nin, int axInfo, >>> ... /* const void* */); >>> NRRD_EXPORT void nrrdAxisInfoGet(const Nrrd *nrrd, int axInfo, >>> ... /* void* */); >>> And this is probably the only use of var-args that I tolerate now, >>> since its in fact just a wrapper around a sprintf() call. >>> NRRD_EXPORT int nrrdContentSet(Nrrd *nout, const char *func, >>> const Nrrd *nin, const char *format, >>> ... /* printf-style arg list */ ); >>> THE CONSEQUENCES ********************* >>> Here are some consequences of the possible API change. Any code >>> using _nva functions will no longer compile, and the answer is to >>> remove the "_nva". Code indulging in the "worst offender" var- args >>> functions, such as: >>> nrrdMaybeAlloc(nout=nrrdNew(), otype, 4, >>> ansLen, sox, soy, soz); >>> will *usually* also have compilation errors, because the function >>> would (with the API change) be declared as: >>> nrrdMaybeAlloc(Nrrd *nrrd, int type, unsigned int dim, >>> const size_t *size); >>> However, this code, for a one-dimensional array: >>> nrrdMaybeAlloc(nout=nrrdNew(), otype, 1, ansLen); >>> will NOT generate a compile error, and WILL create garbage results, >>> because nrrdMaybeAlloc will attempt to dereference ansLen. >>> Ultimately, renaming var-args functions does not fix type safety >>> problems. But I think it makes the nrrd API more consistent with >>> the goal of type safety. >> >> >> If this will cause possible errors without compilation errors, then >> we should provide a list of functions that will need to be fixed. >> Perhaps a simple shell script that will tag all changed functions >> with an undefined function: >> >> nrrdMaybeAlloc -> nrrdMaybeAlloc_fixme >> >> This way you can go through these one at a time as the compiler >> catches them after running the script on your source. >> >>> THE PLEA ********************* >>> What do you think of this? >>> Gordon >> >> >> Type safety is nice. BTW, how does printf discover that your >> parameter types don't match? I'm always getting compiler warnings >> about that when I do something stupid. >> >> I'm still curious how the unsigned int's get munged into the wrong >> thing? >> >> Also, are you proposing that we get rid of the variable argument >> functions or simply a renaming? >> >> Thanks, >> James >> >> >> ------------------------------------------------------- >> SF.Net email is sponsored by: >> Tame your development challenges with Apache's Geronimo App Server. >> Download >> it for free - -and be entered to win a 42" plasma tv or your very own >> Sony(tm)PSP. Click here to play: http://sourceforge.net/geronimo.php >> _______________________________________________ >> teem-users mailing list >> tee...@li... >> https://lists.sourceforge.net/lists/listinfo/teem-users > > |