[PATCH] fix build with GCL and gcc-14

[Qian Yun]

With gcc-14 enables "-Wimplicit-function-declaration" by default,
we get this error when building with GCL:

/tmp/fricas-1.3.11/src/lisp/fricas-lisp.c: In function
‘sock_get_string_buf_wrapper’:
/tmp/fricas-1.3.11/src/lisp/fricas-lisp.c:703:10: error: implicit
declaration of function ‘sock_get_string_buf’; did you mean
‘sock_send_string_len’? [-Wimplicit-function-declaration]
703 | return sock_get_string_buf(i, x->st.st_self, j); }
| ^~~~~~~~~~~~~~~~~~~
| sock_send_string_len


We need to add a declaration in the "clines".

But to get the return type match, I also need to change the
return type of "sock_get_string_buf" to int.
Its return value is currently ignored.

Writing this patch makes me realize that we need to improve
"sock_get_string_buf" more:

First, there's discrepancy between GCL and other Lisps:
if the string is bigger than buffer, GCL gives an error,
while other Lisps fill the buffer with part of the string.
But in this case the following operations with be messed up.
Because there is no loop to clear the socket.

Currently we use a buffer of size 10000, hoping it to be big
enough. This should be improved later.

- Qian

[Waldek Hebisch]

On Tue, Sep 24, 2024 at 09:07:18PM +0800, Qian Yun wrote:
> With gcc-14 enables "-Wimplicit-function-declaration" by default,
> we get this error when building with GCL:
>
> /tmp/fricas-1.3.11/src/lisp/fricas-lisp.c: In function
> ‘sock_get_string_buf_wrapper’:
> /tmp/fricas-1.3.11/src/lisp/fricas-lisp.c:703:10: error: implicit
> declaration of function ‘sock_get_string_buf’; did you mean
> ‘sock_send_string_len’? [-Wimplicit-function-declaration]
> 703 | return sock_get_string_buf(i, x->st.st_self, j); }
> | ^~~~~~~~~~~~~~~~~~~
> | sock_send_string_len
>
>
> We need to add a declaration in the "clines".
>
> But to get the return type match, I also need to change the
> return type of "sock_get_string_buf" to int.
> Its return value is currently ignored.

Yes. Please commit.

> Writing this patch makes me realize that we need to improve
> "sock_get_string_buf" more:
>
> First, there's discrepancy between GCL and other Lisps:
> if the string is bigger than buffer, GCL gives an error,
> while other Lisps fill the buffer with part of the string.
> But in this case the following operations with be messed up.
> Because there is no loop to clear the socket.
>
> Currently we use a buffer of size 10000, hoping it to be big
> enough. This should be improved later.

Yes, this is known problem. Actually, it affects not only
'sock_get_string_buf' but also input from sockets. One
possible solution is to have a loop at Lisp/Boot/Spad
level to get parts of input into a buffer, enlarging it
when needed. In case of sockets we can send size first,
so two iterations would suffice. In case of standard input
we know how big the line is only after reading it.

One more thing: we should really get rid of current uses
of 'sock_get_string_buf', instead we should get input from
a socket. Namely, current protocol of sending control
information via socket, but data via standard input/output
if inherently problematic (can get out of sync).

--
Waldek Hebisch

[Qian Yun]

On 9/29/24 12:11 AM, Waldek Hebisch wrote:
> On Tue, Sep 24, 2024 at 09:07:18PM +0800, Qian Yun wrote:
>
>> Writing this patch makes me realize that we need to improve
>> "sock_get_string_buf" more:
>>
>> First, there's discrepancy between GCL and other Lisps:
>> if the string is bigger than buffer, GCL gives an error,
>> while other Lisps fill the buffer with part of the string.
>> But in this case the following operations with be messed up.
>> Because there is no loop to clear the socket.
>>
>> Currently we use a buffer of size 10000, hoping it to be big
>> enough. This should be improved later.
>
> Yes, this is known problem. Actually, it affects not only
> 'sock_get_string_buf' but also input from sockets. One

I'm confused, isn't input from sockets currently handled by
'sock_get_string_buf'?

> possible solution is to have a loop at Lisp/Boot/Spad
> level to get parts of input into a buffer, enlarging it
> when needed. In case of sockets we can send size first,
> so two iterations would suffice. In case of standard input
> we know how big the line is only after reading it.
>
> One more thing: we should really get rid of current uses
> of 'sock_get_string_buf', instead we should get input from
> a socket. Namely, current protocol of sending control
> information via socket, but data via standard input/output
> if inherently problematic (can get out of sync).
>

Can you elaborate more on this?

- Qian

[Waldek Hebisch]

On Sun, Sep 29, 2024 at 08:40:40AM +0800, Qian Yun wrote:
>
>
> On 9/29/24 12:11 AM, Waldek Hebisch wrote:
> > On Tue, Sep 24, 2024 at 09:07:18PM +0800, Qian Yun wrote:
> >
> > > Writing this patch makes me realize that we need to improve
> > > "sock_get_string_buf" more:
> > >
> > > First, there's discrepancy between GCL and other Lisps:
> > > if the string is bigger than buffer, GCL gives an error,
> > > while other Lisps fill the buffer with part of the string.
> > > But in this case the following operations with be messed up.
> > > Because there is no loop to clear the socket.
> > >
> > > Currently we use a buffer of size 10000, hoping it to be big
> > > enough. This should be improved later.
> >
> > Yes, this is known problem. Actually, it affects not only
> > 'sock_get_string_buf' but also input from sockets. One
>
> I'm confused, isn't input from sockets currently handled by
> 'sock_get_string_buf'?

Sorry, I have messed up things. Normally FriCAS command line
goes via command line editor (rlwrap or clef) to spadclient.
I would have to check if it goes via sman, but eventually
goes to FRICASsys where it is received by 'read_line'.
We should get rid of 'read_line'. We receive strings from
hyperdoc via 'sock_get_string_buf' which goes trough sockets,
but without good protocol.

> > possible solution is to have a loop at Lisp/Boot/Spad
> > level to get parts of input into a buffer, enlarging it
> > when needed. In case of sockets we can send size first,
> > so two iterations would suffice. In case of standard input
> > we know how big the line is only after reading it.
> >
> > One more thing: we should really get rid of current uses
> > of 'sock_get_string_buf', instead we should get input from
> > a socket. Namely, current protocol of sending control
> > information via socket, but data via standard input/output
> > if inherently problematic (can get out of sync).
> >
>
> Can you elaborate more on this?

I mean that currently we just copy bytes, without extra
structure. We should switch to some protocol sending first
info about kind of data, then size and finally the data
itself. And when working with sman we should consistently
use sockets (currently we also get data from standard input
and use standard output). In a sense parts of communication
system are OK now, but we need to make this consistent.
Also, I think that we need to handle splitting of long
lines at protocol level (current code assumes that line
will arrive as one piece, but it is easier to use fixed size
buffers so we need splitting at the source and possibly
reassembly at destination).

--
Waldek Hebisch

[Qian Yun]

Thanks, much clearer now.

Both points (unified IO through socket; structured IO protocol)
are good ideas.

- Qian