Comment 23 for bug 1401454

Revision history for this message
In , Om-brahmana (om-brahmana) wrote :

(In reply to comment #21)
> Created an attachment (id=428218) [details]
> It fixes the issue of already existing directory.please comment
>
> Final patch with Test Case

bsmedberg will do the full review. Here are some of my observations.

This patch only takes care of UNIX and BEOS. What about other operating systems? Though the bug is filed against Linux, I believe we want the behavior to be consistent across operating systems.

>+ PRBool wr = PR_FALSE, ex = PR_FALSE;

Use more descriptive variable names. Example : http://mxr.mozilla.org/mozilla-central/source/xpcom/io/nsLocalFileWin.cpp#2017

>+ wr = (per == 0700)?PR_TRUE:PR_FALSE;

Use mnemonics instead of hard-coding the numerical value : http://mxr.mozilla.org/mozilla-central/source/nsprpub/pr/include/prio.h#652

Also, when you submit a new patch, mark your older versions of the same patch as obsolete.