Hi, I'm going through open patches in patchwork, and noticed the argv stuff - I remembered that David had reviewed it, but going through the mails now I can see that we have no ACK yet, because there are changes needed.
On Mon, Nov 13, 2017 at 12:49:27PM +0100, David Sommerseth wrote:
> Thanks a lot for putting a renewed effort into this. I've reviewed
> this and compile tested with GCC 4.8.5, 6.3.1 and 7.2.1 - as there
> are some warnings appearing; I'll come back to them in a bit.
[..]
> > +argv_printf_arglist(struct argv *a, const char *format, va_list arglist)
> > +{
> > + struct gc_arena gc = gc_new();
> > + const char *delim = 035;
>
> argv.c:217:25: warning: initialization makes pointer from integer without a
> cast [-Wint-conversion]
> const char *delim = 035;
>
> argv.c:227:34: warning: passing argument 2 of ???argv_prep_format??? makes
> integer from pointer without a cast [-Wint-conversion]
> f = argv_prep_format(format, delim, &argc, &gc);
> ^~~~~
This really is a bug. It's declared to be a pointer, but it is used to
just transport "an integer value" (035). It is valid C, but the compiler
is correct in questioning our motives...
> Shouldn't *delim be just delim? I'd propose using:
>
> const char delim = 0x1D; /* ASCII group separator character */
>
> This also kills all the warnings above.
Yes.
Heiko, can you do a v3 round of the remaining argv patches, taking David's
comments into account? Then we can get it into master and finish this
work...
thanks!
gert
--
"If was one thing all people took for granted, was conviction that if you
feed honest figures into a computer, honest figures come out. Never doubted
it myself till I met a computer with a sense of humor."
Robert A. Heinlein, The Moon is a Harsh Mistress
Gert Doering - Munich, Germany g...@greenie.muc.de
signature.asc
Description: PGP signature
_______________________________________________ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
