Hopefully nearly finishing this off now...! 2008/7/16 Ludovic Courtès <[EMAIL PROTECTED]>:
>>> Likewise, `scm_c_read ()' might raise an exception, so we may need a >>> `dynwind' here (my original patch for `uniform-vector-read!' did that). >> >> This is covered by the dynwind inside scm_c_read, isn't it? > > 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: " You must take care to always unreserve an array after reserving it, also in the presence of non-local exits. To simplify this, reserving and unreserving work like a dynwind context (*note Dynamic Wind::): a call to `scm_array_get_handle' can be thought of as beginning a dynwind context and `scm_array_handle_release' as ending it. When a non-local exit happens between these two calls, the array is implicitely unreserved. 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? > My post also contained a `uniform-vector-read!' benchmark, which showed > a noticeable performance improvement on unbuffered ports. Could you try > (and commit) that also? Yes, I'll do that shortly (before and separately from the libguile code changes). > And now for some gratuitous (but friendly) nitpicking... > >> +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? In this case I don't think it's important to "treat that definition as a defun" - and also there are lots of other examples in libguile where the opening brace is on the same line... > 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. */ >> + /* 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! Regards, Neil