Hi bluhm,

Thanks for your reply! I think your method is better, and I update the
patch:

Index: netcat.c
===================================================================
RCS file: /cvs/src/usr.bin/nc/netcat.c,v
retrieving revision 1.192
diff -u -p -r1.192 netcat.c
--- netcat.c    10 Aug 2018 17:15:22 -0000      1.192
+++ netcat.c    6 Sep 2018 10:10:07 -0000
@@ -564,8 +564,11 @@ main(int argc, char *argv[])
                }
                /* Allow only one connection at a time, but stay alive. */
                for (;;) {
-                       if (family != AF_UNIX)
+                       if (family != AF_UNIX) {
+                               if (s == -1)
+                                       close(s);
                                s = local_listen(host, uport, hints);
+                       }
                        if (s < 0)
                                err(1, NULL);
                        if (uflag && kflag) {
@@ -622,9 +625,7 @@ main(int argc, char *argv[])
                                }
                                close(connfd);
                        }
-                       if (family != AF_UNIX)
-                               close(s);
-                       else if (uflag) {
+                       if (family == AF_UNIX && uflag) {
                                if (connect(s, NULL, 0) < 0)
                                        err(1, "connect");
                        }

Thanks!

On 9/6/2018 5:07 AM, Alexander Bluhm wrote:
> On Tue, Sep 04, 2018 at 01:01:38PM +0800, Nan Xiao wrote:
>> Before netcat program exits, it will check whether s is -1, and close
>> socket if s is not -1:
>>
>>      if (s != -1)
>>              close(s);
>>
>> The following patch fixes the issue that netcat will close socket twice
>> if it works as a server:
> 
> I think it is a bug, but should be fixed differently.  The netstat
> code has a lot of if and else for all the use cases.  Adding a s =
> -1 at one place makes it even more confusing.
> 
> In general main() does not reset s to -1, it isets it at the
> beginning.  Look at the client side.  There we close at the beginning
> of the loop:
> 
>               /* Cycle through portlist, connecting to each port. */
>               for (s = -1, i = 0; portlist[i] != NULL; i++) {
>                       if (s != -1)
>                               close(s);
> 
> So on the server side we should do the same, otherwise the code
> will get more and more messy.  Use the same concept everywhere.
> I think this would be better:
> 
>               /* Allow only one connection at a time, but stay alive. */
>               for (;;) {
>                       if (family != AF_UNIX) {
>                               if (s != -1)
>                                       close(s);
>                               s = local_listen(host, uport, hints);
>                       }
> 
> bluhm
> 

-- 
Best Regards
Nan Xiao(肖楠)

Reply via email to