bug-gawk
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Aharon Robbins
Subject: Re: [bug-gawk] [PATCH] gawk 4.1.1 replace "/inet" with preprocessor macro
Date: Fri, 18 Apr 2014 11:07:27 +0300
User-agent: Heirloom mailx 12.5 6/20/10

Hi Andy.

Thanks for this work. A few stylistic suggestions below.

> Please find attached a proposed patch.  I ran one test, and it seems
> to work for me.  Note the difference in behavior:

I think the difference in error messages is OK.

> I ran valgrind on it, and I found this error.  However, it also occurs
> with gawk 4.1.0.  Is this a bug in glibc?

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

On to your patch.

| +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?

| +     {
| +             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.

Otherwise, I think that's it.

Thanks!

Arnold



reply via email to

[Prev in Thread] Current Thread [Next in Thread]