Hi!
Mark H Weaver <m...@netris.org> skribis:
> l...@gnu.org (Ludovic Courtès) writes:
>>>> Mark H Weaver <m...@netris.org> skribis:
>>>>> +set_per_port_read_option (SCM port, int shift, int value)
>>>>
>>>> Also change ‘shift’ to ‘option’, and ‘int value’ to something like
>>>> ‘enum t_option_state value’, where:
>>>>
>>>> enum t_option_state
>>>> {
>>>> OPTION_INHERITED, /* global option setting inherited */
>>>> OPTION_DISABLED,
>>>> OPTION_ENABLED
>>>> };
>>>>
>>>> the goal being to hide as much of the bit-twiddling as possible.
[...]
>> Thus, I thought we’d logically have these 3 functions:
>> set_port_read_options, port_read_options, and applicable_read_options.
>
> Logically, I agree that this would be a nice interface. The problem is
> really one of efficiency. It's quite expensive to access the per-port
> read options directly, because it requires locking the port table mutex,
> doing a hash table lookup, and then an alist lookup. That's not
> something I want to do more than once per call to 'read'. (Even doing
> it once is slightly painful).
Understood.
> Efficiency is the main reason that I chose to compute all of the
> applicable read options and place them in OPTS at the start of 'read'.
> Efficiency is also the reason that I packed all of the read option
> overrides into a single integer.
Yes, that’s fine with me, as long as the visible interface maps as close
as possible to the underlying concepts.
>> Whether these are implemented in terms of bit fields is not the first
>> thing I want to see when I open read.c.
>>
>> Perhaps this is just a matter of presentation, but my impression was
>> that set_port_read_options and the various constants would force me to
>> think in terms of bit-twiddling more than in terms or read options.
>
> FWIW, all of the details of the bit-twiddling and the storage mechanism
> of per-port read options are confined to just two static functions:
> 'init_read_options' and 'set_per_port_read_option'.
>
> The rest of read.c needn't think about bit-twiddling at all. The
> relevant interface for the rest of read.c is as follows:
>
> * Look up applicable read options in OPTS.
> * Set per-port read options by calling 'set_per_port_*'.
OK. I’ll comment on the new version of your patches, thanks!
Ludo’.
