Re: [PATCH] Add a `read' method for ports

[Ludovic Courtès]

Hi Neil,

"Neil Jerram" <address@hidden> writes:

> Hopefully nearly finishing this off now...!

Great, thanks!

> 2008/7/16 Ludovic Courtès <address@hidden>:

>> No, HANDLE must be released here (see my original patch).
>
> I don't think that's right; see "Accessing Arrays from C" in the
> manual, especially:

[...]

>    That is, you need to properly pair reserving and unreserving in your
> code, but you don't need to worry about non-local exits.
> "
>
> Now, as it happens, the code doesn't actually implement what the
> manual says, and in fact scm_array_handle_release() is currently a
> no-op!  But I believe the manual's intention is sensible, so it think
> I think we should commit this patch as-is for now, and raise a bug to
> track the fact that the array/handle API isn't fully implemented.
>
> What do you think?

I'd prefer fixing the manual rather than `scm_array_get_handle ()',
because (1) setting up a dynwind is "costly" (involves some consing),
and (2) I don't know of any other function that does a dynwind behind
the scenes (IOW, let's not break the "rule of least surprise").

OTOH, it may be the case that people have been relied on the described
behavior, in which case it would be wiser to fix `scm_array_get_handle ()' 
to conform to the manual...

>>> +struct port_and_swap_buffer {
>>
>> Please follow the GNU style.  :-)
>
> Well, they say "The open-brace that starts a struct body can go in
> column one if you find it useful to treat that definition as a
> defun.".  Is that what you meant?

Yes, but well...

>> A small comment above might be nice.
>
> Right, added:
>
> /* This structure, and the following swap_buffer function, are used
>    for temporarily swapping a port's own read buffer, and the buffer
>    that the caller of scm_c_read provides. */

Great.

>>> +  /* Reinstate the port's normal buffer. */
>>
>> I like adding a space after periods at the end of a sentence.
>
> But there is a space there - so I don't understand!

I meant two spaces, to distinguish periods that end a sentence from
abbreviations.

Thanks,
Ludo'.