931703 - Adding ENOSPC and a few other POSIX error macros to nsresultForErrno()

Status: RESOLVED FIXED

(Core Graveyard :: File Handling, defect)

Description

[ISHIKAWA, Chiaki]

While trying to fix "Bug 930397 - Failure to handle error during
writing to an almost full file system (TB saving an attachment/FF
downloading)", I noticed that the error of failing PR_Write() is not
propagated properly to callers in nsLocalFile::CopyToNative (File:
mozilla/xpcom/io/nsLocalFileCommon.cpp ) This makes it impossible for
the upper layer caller to display meaningful error message to the
user. Currently, a bland catch-all message ("The download cannot be
saved because an unknown error occurred.") is shown and users are left
with no clue to the cause of the error.

But then, we need a mapping of POSIX errno to NS_ERROR_* type macros
to pass proper error information to the callers.

In mozilla/xpcom/io/nsLocalFileCommon.cpp, I noticed that
nsDownload::CopyMoveFailed() used a macro

    #define NSRESULT_FOR_ERRNO() nsresultForErrno(errno)

defined in
FILE: mozilla/xpcom/io/nsLocalFile.h
to map the value of errno to NS_ERROR_* macros.

So I thought I could use this macro when write failed to write
expected number of octets to a local file (is the typical symptom of
failure to the filled up file system during writing.)

To my dismay, I found out that the macro, and nsresultForErrno()
function in turn, does not implement all the POSIX error values :-(

I do NOT NEED ALL the POSIX error macros, I only need macros for often
seen errors to be handled by nsresultForErrno(errno) for now.

Specifically, for an error scenario, in Bug 930397, I definitely need
the mapping of ENOSPC (e.g., defined under linux as follows.)

    #define	ENOSPC		28	/* No space left on device */

So I decided to add handling of some additional POSIX error macro
values to nsresultForErrno(). In adding ENOSPC to the values handled
nsresultForErrno(), A few more POSIX error macros that may be relevant
to FILE I?O error handling are added also.


A patch to the function follows.

TIA


PS: In doing the above, I noticed a rather unfortunate misguided usage
of NS_ERROR_FILE_TOO_BIG by createUnique() function: I reported the
bug and proposed a patch so that NS_ERROR_FILE_ALREADY_EXISTS is used
instead : Bug 931689 - Strange use of NS_ERROR_FILE_TOO_BIG by
nsLocalFile::CreateUnique: NS_FILE_ALREADY_EXISTS should be used
instead.

With the change in Bug 931689, I can universally convert POSIX EFBIG
to NS_ERROR_FILE_TOO_BIG for the whole the comm-central tree (and thus
the same in the mozilla source tree.)

PPS: this is valid for all POSIX-compliant OSs including MacOSX, I think.
(Or solaris and FreeBSD for that matter.)

Comment 1

[ISHIKAWA, Chiaki]

Attachment: PATCH: Adding the processing of a few POSIX errors macros.

This is the patch.

The lack of handling of these important additional POSIX error macros may explain the lack of user-friendly detailed error handling in TB (and FF maybe, too.)

I hope this gets in soon. But I am not sure who the reviewer should be.

TIA

Comment 2

[Benjamin Smedberg]

Comment on attachment 823226 [details] [diff] [review]
PATCH: Adding the processing of a few POSIX errors macros.

r=me except for the comment about CreateUnique, which needs to continue returning NS_ERROR_FILE_TOO_BIG.

Comment 3

[ISHIKAWA, Chiaki]

Attachment: FINAL PATCH: Adding the processing of a few POSIX errors macros. (carrying over r= bsmedberg)

Thank you for the review.

I have changed the description to be a note for the newcomer. (I think a comment is necessary here to avoid future misunderstanding.)

If this is OK, then I will set checkin-needed keyword.

TIA

Comment 4

[ISHIKAWA, Chiaki]

Thank you for the review.
I will put in the checkin-needed keyword.

Comment 5

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/61d3b1f4614c

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/61d3b1f4614c