Date: Sat, 2 Aug 2008 13:15:38 -0700 From: "Garrett Cooper" <yanegomi@gmail.com> To: "Tim Kientzle" <kientzle@freebsd.org> Cc: Anselm Strauss <strauss@freebsd.org>, Perforce Change Reviews <perforce@freebsd.org> Subject: Re: PERFORCE change 146324 for review Message-ID: <364299f40808021315m7fcb67d4pf2ef3bc4612f9f1d@mail.gmail.com> In-Reply-To: <4894AC63.8070403@freebsd.org> References: <200807311508.m6VF8QUD097494@repoman.freebsd.org> <4894AC63.8070403@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Sat, Aug 2, 2008 at 11:50 AM, Tim Kientzle <kientzle@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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?364299f40808021315m7fcb67d4pf2ef3bc4612f9f1d>