1102709 - The implementation of _PR_MD_TLOCKFILE and _PR_MD_LOCKFILE should be exchanged in w95io.c

Status: NEW

(NSPR :: NSPR, defect)

Description

[JW Wang [:jwwang] [:jw_wang]]

See http://msdn.microsoft.com/en-us/library/windows/desktop/aa365202(v=vs.85).aspx

If LockFile cannot lock a region of a file, it returns zero immediately. It does not block. To issue a file lock request that will block until the lock is acquired, use LockFileEx without the LOCKFILE_FAIL_IMMEDIATELY flag.

Comment 1

[JW Wang [:jwwang] [:jw_wang]]

Attachment: 1102709_exchange_PR_MD_LOCKFILE_and_PR_MD_TLOCKFILE.patch

exchange the implementation of _PR_MD_TLOCKFILE and _PR_MD_LOCKFILE.

Comment 2

[Wan-Teh Chang]

Comment on attachment 8526583 [details] [diff] [review]
1102709_exchange_PR_MD_LOCKFILE_and_PR_MD_TLOCKFILE.patch

Review of attachment 8526583 [details] [diff] [review]:
-----------------------------------------------------------------

Thank you for the patch. I suggest some improvements.

::: pr/src/md/windows/w95io.c
@@ +947,5 @@
>  
>  PRStatus
>  _PR_MD_LOCKFILE(PROsfd f)
>  {
> +    PR_SetError( PR_NOT_IMPLEMENTED_ERROR, 0 );

Could you implement this function using LockFileEx?

@@ +959,5 @@
> +    DWORD     rv;
> +
> +    rv = LockFile( (HANDLE)f,
> +        0l, 0l,
> +        0x0l, 0xffffffffl );

Nit: please put all the arguments on one line.

@@ +961,5 @@
> +    rv = LockFile( (HANDLE)f,
> +        0l, 0l,
> +        0x0l, 0xffffffffl );
> +    if ( rv == 0 ) {
> +        DWORD rc = GetLastError();

I have a patch for this line because this |rc| variable
hides the |rc| variable declared in the outer scope.

Could you please review my patch in bug 1089908? Or
you can incorporate my patch into this patch? Note that
I also added a _PR_MD_MAP_DEFAULT_ERROR call to map the
Windows error code to an NSPR error code.

Comment 3

[JW Wang [:jwwang] [:jw_wang]]

Thanks. I will wait for bug 1089908 and try to implement _PR_MD_LOCKFILE.

Comment 4

[(not currently active) Ted Mielczarek]

Comment on attachment 8526583 [details] [diff] [review]
1102709_exchange_PR_MD_LOCKFILE_and_PR_MD_TLOCKFILE.patch

Review of attachment 8526583 [details] [diff] [review]:
-----------------------------------------------------------------

Sounds like Wan-Teh has this. Flag me for review if you need anything else.