On 30 June 2014 19:43, Darrick J. Wong <darrick.wong@xxxxxxxxxx> wrote:
>> + retval = ext2fs_xattrs_open(fs, ino, &handle);
>
> If the FS does not have at least one of the inline_data or ext_attr features
> turned on, the ext2fs_xattrs_open call returns EXT2_ET_MISSING_EA_FEATURE,
> which aborts the whole operation. Is that ok?
Good point.
I'm currently dithering over whether this should silently do nothing,
or warn per file, or have an already-warned boolean to avoid spamming
the user.
>> + if (retval) {
>> + com_err(__func__, errno, "while opening inode %u", ino);
>
> retval, not errno.
Literally just squashing some commits for these error paths which went wrong.
>> + return errno;
>> + }
>> +
>> + list = malloc(size);
>
> What happens if malloc fails?
You're doomed?
What's e2fsprog's policy on OOM - I see some instances of checked
malloc() that exit() promptly, others that simply return 0 and hope
for the best, and some that use fputs() and gettext() - but if we're
OOM then that's even more likely to fail.
I'll follow the official line, but what that line is isn't clear from
a sample of malloc() calls in the source.
>> + size = llistxattr(filename, list, size);
>
> What if this second call should fail for some reason? Shouldn't we stop?
Yes, also fixed locally.
>> +
>> + for (i = 0; i < size; i += strlen(&list[i]) + 1) {
>> + const char *name = &list[i];
>> + char *value;
>> +
>> + value_size = getxattr(filename, name, NULL, 0);
>
> What if getxattr returns -1?
Should bail, will fix.
>> + value = malloc(value_size);
>> + value_size = getxattr(filename, name, value, value_size);
>
> Same complaints about not checking malloc/getxattr return values.
Agreed.
>> + retval = ext2fs_xattrs_close(&handle);
>> + if (retval)
>> + com_err(__func__, errno, "while closing inode %u", ino);
>
> retval, not errno.
[sags head in shame]
Next time I'm patching at 10pm I'll remember to review the patch the
next morning before sending...
Ross
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
