Comment 19 for bug 96566

Revision history for this message
In , Mnyromyr (mnyromyr) wrote :

Comment on attachment 385658
Patch, version 1

Nice, just some nits:

>diff --git a/mailnews/local/src/nsMovemailService.cpp b/mailnews/local/src/nsMovemailService.cpp
> PRBool ObtainSpoolLock(const char *spoolnameStr,
>- int seconds /* number of seconds to retry */)
>+ int seconds /* number of seconds to retry */,
>+ PRBool *usingLockFile)
> {

While you're touching these functions, you could make the parameters adhere to the Mozilla 'aSpoolName' naming scheme. (Same for YieldSpoolLock down below.)
And you should check
  NS_ENSURE_ARG_POINTER(aUsingLockFile);
here.

>- // How to lock:
>- // step 1: create SPOOLNAME.mozlock
>- // 1a: can remove it if it already exists (probably crash-droppings)
>- // step 2: hard-link SPOOLNAME.mozlock to SPOOLNAME.lock for NFS atomicity
>- // 2a: if SPOOLNAME.lock is >60sec old then nuke it from orbit
>- // 2b: repeat step 2 until retry-count expired or hard-link succeeds
>- // step 3: remove SPOOLNAME.mozlock
>- // step 4: If step 2 hard-link failed, fail hard; we do not hold the lock
>- // DONE.
>- //
>- // (step 2a not yet implemented)

Please don't let that comment die, just move it down after the new kernel locking.

>+ nsresult rv = NS_NewNativeLocalFile(nsDependentCString(spoolnameStr), PR_TRUE,
>+ getter_AddRefs(spoolFile));

Either use one line, ignoring its length, or put arguments 2 and 3 on their own line and align them vertically with nsDep...
(Plus one more instance further down.)

>+ if (!*usingLockFile)
>+ {

Please adhere to the prevalent bracing style of this file.
(Plus some more instances further down.)