Re: patch - fixing sample program in unix.7

[Alejandro Colomar <alx@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 5224 bytes --]

[CC += Heinrich]

Hi Lili,

On Mon, Mar 04, 2024 at 05:41:17PM +0100, Lili Püspök wrote:
> Hi Alejandro,
> Thanks for replying.
> 
> The client, after connecting, processes the argv items and sends all
> of them, then issues the sending of a final END which, on the server
> side, is not expected after DOWN which would halt the reading (In that
> case, too, the processing of argv + the END should happen).
> After the change,  DOWN does not break the reading, the closing END is
> processed and there is no broken connection when client tries to send
> END while the server closes after sending the result, which is not
> received by the client.

Hmmm, now I understand.

> 
> --- without DOWN ----
> client         server
> argv1..N + END --->
> <----- result
> <---- connection closed
> 
> -----------problem-----------
> argv1...N + DOWN ->
> <---- result
> END -> ?????
> <---- connection closed
> 
> ------- solution:---------
> argv1...N + DOWN + END ->
> <---- result
> <---- connection closed

Yep, I can reproduce this problem all the way back to the original
implementation of the example programs.  I extracted the original
programs with:

	$ git show 15545eb6d7:man7/unix.7 | man /dev/stdin | cat

And then cut and paste to the C files.

	$ cc -Wall -o server server.c 
	$ cc -Wall -o client client.c 
	$ ./server &
	[1] 94644
	$ ./client 3 4
	Result = 7
	$ ./client 11 -5
	Result = 6
	$ ./client DOWN
	recv: Connection reset by peer
	[1]+  Done                    ./server
	$ 

This behavior conflicts with the behavior shown in the manual page,
which shows (for the last command):

	$ ./client DOWN
	Result = 0
	[1]+  Done                    ./server

So it seems like a bug.  Maybe the server program was slow enough when
it was implemented in 2016, that the server hadn't closed the socket
when the client sent "END", so the client didn't fail to read the
result??

Anyway, we need to fix it.  Agree.

Please add

	Fixes: 15545eb6d7ae ("unix.7: Add example")
	Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>

to the commit message, as well as a small description of the problem.

> I hope I did not overlook something.

However, I'm not convinced by your patch.  It seems to allow

	$ ./client DOWN 4 3

which I don't think we want to support.  I think we have two options:

-  The client avoids sending "END" after "DOWN" (so DOWN implies END).
-  The server only accepts "END" after "DOWN".

Does it make sense to you?

Have a lovely day!
Alex

> OK, maybe with a unique message containing only the DOWN from client,
> the issue is not visible, because there is no result to return to
> client and we don't care it the connection is just broken....
> 
> Cheers
> PuLi
> 
> Alejandro Colomar <alx@kernel.org> ezt írta (időpont: 2024. márc. 4., H, 17:22):
> >
> > Hi Lili,
> >
> > > Subject: Re: patch - fixing sample program in unix.7
> >
> > On Sun, Mar 03, 2024 at 08:27:17PM +0100, Lili Püspök wrote:
> > > diff --git a/man7/unix.7 b/man7/unix.7
> >
> > Please add some commit message.  I don't understand what this patch
> > does.  How is it broken, and how does it fix it?
> >
> > > index cb1dcae45..7fb41af99 100644
> > > --- a/man7/unix.7
> > > +++ b/man7/unix.7
> > > @@ -1057,7 +1057,7 @@ main(int argc, char *argv[])
> > > \&
> > >             if (!strncmp(buffer, "DOWN", sizeof(buffer))) {
> > >                 down_flag = 1;
> > > -                break;
> > > +                continue;
> >
> > DOWN is used to stop the server.  How would 'continue' help?
> >
> >
> >         $ MANWIDTH=66 man unix | grep -C2 DOWN
> >              tegers.  The client prints the sum and exits.   The  server
> >              waits  for the next client to connect.  To stop the server,
> >              the client is called with the command‐line argument "DOWN".
> >
> >              The following output was recorded while running the  server
> >              in the background and repeatedly executing the client.  Ex‐
> >              ecution  of  the  server  program ends when it receives the
> >              "DOWN" command.
> >
> >            Example output
> >         --
> >                  $ ./client 11 -5
> >                  Result = 6
> >                  $ ./client DOWN
> >                  Result = 0
> >                  [1]+  Done                    ./server
> >         --
> >                          /* Handle commands. */
> >
> >                          if (!strncmp(buffer, "DOWN", sizeof(buffer))) {
> >                              down_flag = 1;
> >                              break;
> >         --
> >                      close(data_socket);
> >
> >                      /* Quit on DOWN command. */
> >
> >                      if (down_flag) {
> >
> > Have a lovely day,
> > Alex
> >
> >
> > >             }
> > > \&
> > >             if (!strncmp(buffer, "END", sizeof(buffer))) {
> > >
> >
> > --
> > <https://www.alejandro-colomar.es/>
> > Looking for a remote C programming job at the moment.

-- 
<https://www.alejandro-colomar.es/>
Looking for a remote C programming job at the moment.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]