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