Re: [bug-gawk] [PATCH] gawk 4.1.1 replace "/inet" with preprocessor macro

[Andrew J. Schorr]

Hi Arnold,

On Fri, Apr 18, 2014 at 11:07:27AM +0300, Aharon Robbins wrote:
> I think the difference in error messages is OK.

I agree.  I should point out one more difference in this regard.
With the old code:

bash-4.2$ gawk 1 /inet/tcp/0/blah/fubar/junk
gawk: fatal: remote host and port information (blah, fubar/junk) invalid

And with the patched code:

bash-4.2$ gawk 1 /inet/tcp/0/blah/fubar/junk
gawk: fatal: cannot open file `/inet/tcp/0/blah/fubar/junk' for reading (No 
such file or directory)

If it's possible for a valid <remote port> specification to contain one or
more "/" characters, then I need to revise my patch.  But I don't think that's
allowed; am I wrong?

> Looks like it to me. It's not the only glibc bug that valgrind turns up. :-)

I wrote a trivial C program to call getaddrinfo, and I have confirmed that
it's a glibc bug.

> | +struct inet_socket_info {
> | +   int family;             /* AF_UNSPEC, AF_INET, or AF_INET6 */
> | +   int protocol;           /* SOCK_STREAM or SOCK_DGRAM */
> | +   struct {
> | +           int offset;
> | +           int len;
> | +   } localport, remotehost, remoteport;
> | +};
> 
> I'm thinking it'd make more sense to have the 'offset' member be
> a char pointer directly into the relevant part of the string. Doing
> so would simplify the uses.
> 
> Thoughts?

I did this for a very specific reason.  My initial plan was to store
pointers rather than offsets, but I did not know whether to make
the type 'const char *' or 'char *'.  The inetfile() function is called
with a 'const char *' name to parse.  So one would naturally use 'const char *'
pointers for the components of the name.  But the devopen() function,
which also takes a 'const char *name' argument, casts it to a 'char *'
in order to change a const value.  I frankly did not want to participate
in this evil act.  If you compile io.c with -Wcast-qual, you will see this:

io.c: In function ?devopen?:
io.c:1570:8: warning: cast discards ?__attribute__((const))? qualifier from 
pointer target type [-Wcast-qual]
   cp = (char *) name + 5;
        ^
io.c:1592:8: warning: cast discards ?__attribute__((const))? qualifier from 
pointer target type [-Wcast-qual]
   cp = (char *) name;
        ^
In file included from io.c:31:0:
io.c: In function ?get_read_timeout?:
io.c:3849:11: warning: cast discards ?__attribute__((const))? qualifier from 
pointer target type [-Wcast-qual]
     efree((void *) last_name);
           ^
awk.h:1272:23: note: in definition of macro ?efree?
 #define efree(p) free(p)
                       ^
I didn't want to write any code that would cause more violations, so I chose
to use offsets to avoid adding more illegal casts.

If you would prefer to avoid offsets, please let me know what types you
would like to use.

> | +   {
> | +           struct inet_socket_info isi;
> | +           if (inetfile(str, & isi)) {
> | +                   tflag |= RED_SOCKET;
> | +                   if (isi.protocol == SOCK_STREAM)
> | +                           tflag |= RED_TCP;       /* use shutdown when 
> closing */
> | +           }
> 
> Can you please move the declaration of isi up to the top of the
> function? That removes the need for the braces and the extra indentation.

I made that change.  An updated version is attached.

Regards,
Andy

inetfile.patch
Description: Text document