On Mon, Sep 15, 2008 at 01:23:39PM +0800, Wilkinson, Alex wrote:
> 0n Sun, Sep 14, 2008 at 09:28:28PM -0700, Jeremy Chadwick wrote:
>
> >On Mon, Sep 15, 2008 at 10:37:18AM +0800, Wilkinson, Alex wrote:
> >> 0n Fri, Sep 12, 2008 at 09:32:07AM -0700, Jeremy Chadwick wrote:
> >>
> >> >> About the only real improvement I'd like to see in this setup
> is the ability
> >> >> to spin down idle drives. That would be an ideal setup for the
> home RAID
> >> >> array.
> >> >
> >> >There is a FreeBSD port which handles this, although such a
> feature
> >> >should ideally be part of the ata(4) system (as should TCQ/NCQ
> and a
> >> >slew of other things -- some oon 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'.
