[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
- [bug-gawk] [PATCH] gawk 4.1.1 replace "/inet" with preprocessor macro, Dave Sines, 2014/04/14
- Re: [bug-gawk] [PATCH] gawk 4.1.1 replace "/inet" with preprocessor macro, Eli Zaretskii, 2014/04/14
- Re: [bug-gawk] [PATCH] gawk 4.1.1 replace "/inet" with preprocessor macro, Dave Sines, 2014/04/15
- Re: [bug-gawk] [PATCH] gawk 4.1.1 replace "/inet" with preprocessor macro, Andrew J. Schorr, 2014/04/15
- Re: [bug-gawk] [PATCH] gawk 4.1.1 replace "/inet" with preprocessor macro, Dave Sines, 2014/04/15
- Re: [bug-gawk] [PATCH] gawk 4.1.1 replace "/inet" with preprocessor macro, Aharon Robbins, 2014/04/17
- Re: [bug-gawk] [PATCH] gawk 4.1.1 replace "/inet" with preprocessor macro, Andrew J. Schorr, 2014/04/17
- Re: [bug-gawk] [PATCH] gawk 4.1.1 replace "/inet" with preprocessor macro, Aharon Robbins, 2014/04/17
- Re: [bug-gawk] [PATCH] gawk 4.1.1 replace "/inet" with preprocessor macro, Andrew J. Schorr, 2014/04/17
- Re: [bug-gawk] [PATCH] gawk 4.1.1 replace "/inet" with preprocessor macro,
Aharon Robbins <=
- Re: [bug-gawk] [PATCH] gawk 4.1.1 replace "/inet" with preprocessor macro, Andrew J. Schorr, 2014/04/18
- Re: [bug-gawk] [PATCH] gawk 4.1.1 replace "/inet" with preprocessor macro, Aharon Robbins, 2014/04/20
- Re: [bug-gawk] [PATCH] gawk 4.1.1 replace "/inet" with preprocessor macro, Andrew J. Schorr, 2014/04/20