Martin Maechler wrote: [...] > vQ> you return s, which should be the same pointer value (given the actual > vQ> code that does not modify the local variable s) with the same > pointed-to > vQ> string value (given the signature of the function). > > vQ> was perhaps > > vQ> char *elim_trailing(char* const s, char cdec) > > vQ> intended? > > yes that would seem slightly more "logical" to my eyes, > and "in principle" I also agree with the other remarks you make above, >
what does ' "in principle" ' mean, as opposed to 'in principle'? (is it emphasis, or sneer quotes?) > ... > > vQ> anyway, having the pointer s itself declared as const does > vQ> make sense, as the code seems to assume that exactly the input pointer > vQ> value should be returned. or maybe the argument to elim_trailing > should > vQ> not be declared as const, since elim_trailing violates the > declaration. > > vQ> one way out is to drop the violated const in both the actual argument > vQ> and in elim_trailing, which would then be simplified by removing all > vQ> const qualifiers and (char*) casts. > > I've tried that, but ``it does not work'' later: > {after having renamed 'elim_trailing' to 'dropTrailing0' } > my version of *using* the function was > > 1 SEXP attribute_hidden StringFromReal(double x, int *warn) > 2 { > 3 int w, d, e; > 4 formatReal(&x, 1, &w, &d, &e, 0); > 5 if (ISNA(x)) return NA_STRING; > 6 else return mkChar(dropTrailing0(EncodeReal(x, w, d, e, OutDec), OutDec)); > 7 } > > where you need to consider that mkChar() expects a 'const char*' > and EncodeReal(.) returns one, and I am pretty sure this was the > main reason why Petr had used the two 'const char*' in (the > now-named) dropTrailing0() definition. > If I use your proposed signature > > char* dropTrailing0(char *s, char cdec); > > line 6 above gives warnings in all of several incantations I've tried > including this one : > > else return mkChar((const char *) dropTrailing0((char *)EncodeReal(x, w, > d, e, OutDec), OutDec)); > > which (the warnings) leave me somewhat clue-less or rather > unmotivated to dig further, though I must say that I'm not the > expert on the subject char* / const char* .. > of course, if the input *is* const and the output is expected to be const, you should get an error/warning in the first case, and at least a warning in the other (depending on the level of verbosity/pedanticity you choose). but my point was not to light-headedly change the signature/return of elim_trailing and its implementation and use it in the original context; it was to either modify the context as well (if const is inessential), or drop modifying the const string if the const is in fact essential. > vQ> another way out is to make > vQ> elim_trailing actually allocate and return a new string, keeping the > vQ> input truly constant, at a performance cost . yet another way is > to > vQ> ignore the issue, of course. > > vQ> the original (martin/petr) version may quietly pass -Wall, but the > vQ> compiler would complain (rightfully) with -Wcast-qual. > > hmm, yes, but actually I haven't found a solution along your > proposition that even passes -pedantic -Wall -Wcast-align > (the combination I've personally been using for a long time). > one way is to return from elim_trailing a new, const copy of the const string. using memcpy should be efficient enough. care should be taken to deallocate s when no longer needed. (my guess is that using the approach suggested here, s can be deallocated as soon as it is copied, which means pretty much that it does not really have to be const.) > Maybe we can try to solve this more esthetically > in private e-mail exchange? > sure, we can discuss aesthetics offline. as long as we do not discuss aesthetics (do we?), it seems appropriate to me to keep the discussion online. i will experiment with a patch to solve this issue, and let you know when i have something reasonable. best, vQ ______________________________________________ R-devel@r-project.org mailing list https://stat.ethz.ch/mailman/listinfo/r-devel