Filipe Cabecinhas <[email protected]> writes:
> Sorry for the delay. I've updated the patch to work as you suggested (I
> think).
> It's attached.
A few comments.
First, the formalities. Please see Documentation/SubmittingPatches,
notably:
- Please do not send a patch as an attachment.
- The attached patch does not seem to have any proposed commit
log message. For this particular change, I expect it would not
be more than a paragraph of several lines. Please have one.
- Please sign-off your patch.
Look at patches from other high-value contributors to learn the
style and the tone of log message. For instance,
http://article.gmane.org/gmane.comp.version-control.git/223406
is a good example (taken at random).
> diff --git a/compat/write.c b/compat/write.c
> new file mode 100644
> index 0000000..1e890aa
> --- /dev/null
> +++ b/compat/write.c
> @@ -0,0 +1,11 @@
> +#include <limits.h>
> +#include <unistd.h>
> +
> +/* Version of write that will write at most INT_MAX bytes.
> + * Workaround a xnu bug on Mac OS X */
/*
* We format our multi-line comments like this.
*
* Nothing other than slash-asterisk/asterisk-slash
* on the first and the last line of the comment block.
*/
> +ssize_t clipped_write(int fildes, const void *buf, size_t nbyte) {
> + if (nbyte > INT_MAX)
> + return write(fildes, buf, INT_MAX);
> + else
> + return write(fildes, buf, nbyte);
> +}
Style:
- opening and closing parentheses of the function body sit on
lines on their own.
- one level of indent is 8 places, using a tab.
Perhaps it would look more like this (my MUA may clobber the tabs,
though, before it gets to you):
ssize_t clipped_write(int fildes, const void *buf, size_t nbyte)
{
if (nbyte > INT_MAX)
nbyte = INT_MAX;
return write(fildes, buf, nbyte);
}
I do not want to see this ffile called "write.c"; we will encounter
a platform whose write(2) behaves a way that we do not expect but is
different from this "clipped to INT_MAX" in the future. The way we
will deal with such a platform is to add a workaround similar to
this patch somewhere in the compat/ directory, but if you squat on
"compat/write.c", it would be a problem for that platform, as it
cannot call its version "compat/write.c".
Perhaps call it "compat/clipped-write.c" or something.
By the way, does your underlying write(2) correctly write INT_MAX
bytes, or did you mean to clip at (INT_MAX-1)? Just double-checking.
Other than that, the patch looks sensible to me.
Thanks.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
