Hi Waclav (and other interested parties),
I have committed my working version of src/main/coerce.c
so you can prepare your patch against that.
Thank you in advance!
Martin
On Fri, May 29, 2009 at 21:54, Wacek Kusnierczyk
<waclaw.marcin.kusnierc...@idi.ntnu.no> wrote:
> 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
