Re: Making custom binary input ports unbuffered

[Ludovic Courtès]

Mark H Weaver <address@hidden> skribis:

> address@hidden (Ludovic Courtès) writes:
>
>> Mark H Weaver <address@hidden> skribis:
>>
>>> Here are the same changes adapted for master, where we can put the
>>> new 'setvbuf' method where it belongs: in the PTOB.
>>
>> Cool!
>>
>> Wouldn’t it be easier to just merge stable-2.0 and modify from there?
>
> That's initially what I started to do, but merging this patch to master
> was more work than I expected.  There are massive changes to the ports
> code in master.  It involved asking questions like "Are there any new
> ways to create file ports in master?", and involved several merge
> failures for changes that will be immediately and completely undone
> anyway.  I found myself asking "Why am I doing this?"

OK.

> If you want to do it like you suggest, would you be willing to handle
> the merge of your setvbuf patches to master?  I've already merged
> everything else.

I trust your judgment on this.  :-)

>>> From 00ee913e2da658f30d9d8682edfbb9a63719c370 Mon Sep 17 00:00:00 2001
>>> From: Mark H Weaver <address@hidden>
>>> Date: Tue, 21 Jan 2014 01:57:31 -0500
>>> Subject: [PATCH 1/2] Prepare 'setvbuf' to support for non-file ports.
>>> MIME-Version: 1.0
>>> Content-Type: text/plain; charset=UTF-8
>>> Content-Transfer-Encoding: 8bit
>>>
>>> Based on a patch for Guile 2.0 by Ludovic Courtès.
>>>
>>> * libguile/fports.c (scm_fport_buffer_add): Rename to 'fport_setvbuf'.
>>>   (fport_setvbuf): Renamed from 'scm_fport_buffer_add'.  Change type of
>>>   'write_size' argument from 'int' to 'long'.
>>>   (scm_i_fdes_to_port): Adapt to renamed 'scm_fport_buffer_add'.
>>>   (scm_make_fptob): Set 'setvbuf' method to 'fport_setvbuf'.
>>>   (scm_setvbuf): Move to ports.c.
>>>
>>> * libguile/ports.c (scm_make_port_type): Initialize 'setvbuf' field.
>>>   (scm_set_port_setvbuf): New API function.
>>>   (scm_setvbuf): Moved from fports.c.  Accept any open port, and error
>>>   out when the ptob's 'setvbuf' field is NULL.  Remove explicit
>>>   'scm_gc_free' calls.  Call ptob's 'setvbuf' method instead of
>>>   'scm_fport_buffer_add'.
>>>
>>> * libguile/fports.h (scm_setbuf0): Remove prototype for non-existent
>>>   function.
>>>   (scm_setvbuf): Move prototype to ports.h.
>>>
>>> * libguile/ports.h (scm_t_ptob_descriptor): Add 'setvbuf' member.
>>>   (scm_set_port_setvbuf): Add prototype.
>>>   (scm_setvbuf): Move prototype here from fports.h.
>>
>> Moving ‘setvbuf’ to ports.c also needs to be done in stable-2.0 (and
>> it’s worth making it in a separate patch, IMO.)
>
> Okay.  Other changes that could be done in 2.0 include renaming
> 'scm_fport_buffer_add' to 'fport_setvbuf' and removing the prototype for
> 'scm_setbuf0'.

Right.

>>> * libguile/ports-internal.h (struct scm_port_internal): Change
>>>   'pending_eof' to a 1-bit unsigned char.  Add comment for 'alist'
>>>   member.
>>>
>>> * test-suite/tests/ports.test ("setvbuf")["closed port", "string port"]:
>>>   New tests.
>>>
>>> * doc/ref/api-io.texi (Port Implementation): Document new 'setvbuf'
>>>   member of ptob, and 'scm_set_port_setvbuf'.
>>
>> I was wondering if the ‘setvbuf’ method should be exposed at this point,
>> because I wasn’t 100% sure of the API.  WDYT?
>
> Hmm.  I'm not 100% sure of it either.  Unfortunately, we cannot avoid
> exposing it as soon as it goes into the PTOB, since the PTOB structure
> is public.

Argh, right.  I would be ideal if scm_t_ptob_descriptor were private
too, but I guess we cannot really do this now.  Maybe we could deprecate
it for external use in master (with an SCM_DEPRECATED attribute)?

> Can we discuss the API now and reach a decision?

Yes.  (I don’t have much to say though, because it looked OKish, but I
just wasn’t sure if it was the Right Way.)

>> Perhaps it’s also better for a separate patch?
>
> What as a separate patch?  Do you mean that we should augment the PTOB
> in one patch and not document it until a later patch?

I was referring to the addition of ‘scm_set_port_setvbuf’, but I
understand your point now.

Thanks,
Ludo’.