Comment 25 for bug 1401454

Revision history for this message
In , Timeless-bemail (timeless-bemail) wrote :

Comment on attachment 428479
Patch;Please Comment

>+ PRBool isWritable = PR_FALSE, isExecutable = PR_FALSE;

please include spaces around (=):
>+ PRInt16 i=1;

this isn't a particularly good variable name:
>+ PRUint32 per;

>+ if (!user) {

indentation following this line is confused, it should be 4-4, but for some reason you're doing something else:
>+ user = PR_GetEnv("USERNAME");
>+ if (!user || !*user) {
>+ user = PR_GetEnv("USER");
>+ if (!user || !*user) {
>+ user = PR_GetEnv("LOGNAME");

if this fails, things are strange.

>+ }
>+ }
>+ }

>+ nsDependentCString path(tPath);
>+ nsDependentCString tem;

we don't typically use nsDependent*String for string operations:
>+ path += "mozilla-";

instead, you can use ns*(Auto)String
>+ path += user;

>+ rv = NS_NewNativeLocalFile(path,PR_TRUE,aFile);
>+ while (!isExecutable || !isWritable) {

You didn't check isSymLink:
>+ ((nsILocalFile *)*aFile)->Exists(&isExecutable);
>+ if (isExecutable) {

you should check the return value of xpcom method calls. perhaps the function you're calling failed:
>+ ((nsIFile *)*aFile)->GetPermissions(&per);

again, your indentation is strange:
>+ isWritable = (per == PR_IRWXU)?PR_TRUE:PR_FALSE;
>+ if (!isWritable) {

Use .Truncate() instead, SetIsVoid is for very very rare cases:
>+ tem.SetIsVoid(PR_TRUE);

We have a nsIFile.createUnique, you might want to use that instead:
>+ tem.AppendInt(i,10);

generally you should have spaces after commas:
>+ rv = NS_NewNativeLocalFile(tem,PR_TRUE,aFile);

You can't do this, you need to use do_QueryInterface:
>+ ((nsILocalFile *)*aFile)->

What happens if the file system is FAT/NTFS or something similarly exotic?