On Sun, Jun 10, 2012 at 6:11 PM, Romain Francoise <[email protected]> wrote:
> I fixed the issue with the following patch instead, which expresses the
> intent more clearly:
>
> --- a/metaentry.c
> +++ b/metaentry.c
> @@ -401,8 +401,9 @@
> for (i = 0; i < mentry->xattrs; i++) {
> write_string(mentry->xattr_names[i], to);
> write_int(mentry->xattr_lvalues[i], 4, to);
> - write_binary_string(mentry->xattr_values[i],
> - mentry->xattr_lvalues[i],
> to);
> + if (mentry->xattr_lvalues[i])
> +
> write_binary_string(mentry->xattr_values[i],
> +
> mentry->xattr_lvalues[i], to);
> }
> }
> }
Well, Zak's patch title named real-world problem that it was fixing,
but the patch itself fixed origin of the problem.
Your patch is OK from user PoV, but write_binary_string() is still
broken for len == 0. You make a workaround (changing perfectly fine
mentries_tofile()) for the case of valueless xattr entry (i.e. fixing
usage instead of problematic function - xfwrite()), thus making the
code unnecessarily more complex, instead of going with more generic
and in fact simpler solution. Remember that 0 as count in write() is
allowed and not treated as error (it's specified at least for regular
files), so any good custom *write() function should preserve that
feature IMHO.
Regards.
--
Przemysław 'Przemoc' Pawełczyk
http://przemoc.net/
