PERFORCE change 146324 for review

Garrett Cooper yanegomi at gmail.com
Sat Aug 2 20:39:51 UTC 2008


On Sat, Aug 2, 2008 at 11:50 AM, Tim Kientzle <kientzle at freebsd.org> wrote:
> Anselm Strauss wrote:
>>
>> http://perforce.freebsd.org/chv.cgi?CH=146324
>
>>        ret = (a->compressor.write)(a, &h, sizeof(h));
>> -       if (ret != ARCHIVE_OK) return (ARCHIVE_FATAL);
>> +       if (ret != ARCHIVE_OK) {
>> +               archive_set_error(&a->archive, EIO, "Can't write local
>> file header");
>> +               return (ARCHIVE_FATAL);
>> +       }
>
> compressor.write should have already set an error
> code and message if it's returning an error.
> So this isn't needed.  (In fact, it's a bad
> idea.  The writer knows more about the cause
> of the error, and by overwriting the error message,
> you're just losing useful information.  It's
> much more useful, for example, to see "Disk full"
> or "read-only filesystem" than to see "can't
> write Zip header.")

Unless you want to do something like:

"Can't write Zip header.\nReason:\n%s", strerror(errno)

to trace down the failure point?

Does archive_set_error use err*(3)? If so it makes my above comment moot...

-Garrett


More information about the p4-projects mailing list