[ion] 14711: Remove bogus error message. It is normal for Mutex creation to fail since the Object will alredy exist after the first time it's created

Remove bogus error message. It is normal for Mutex creation to fail
since the Object will alredy exist after the first time it's created
Modified: trunk/reactos/lib/rpcrt4/rpcrt4_main.c
  _____

Modified: trunk/reactos/lib/rpcrt4/rpcrt4_main.c
--- trunk/reactos/lib/rpcrt4/rpcrt4_main.c	2005-04-20 12:46:39 UTC
(rev 14710)
+++ trunk/reactos/lib/rpcrt4/rpcrt4_main.c	2005-04-20 12:52:13 UTC
(rev 14711)
@@ -152,8 +152,6 @@

case DLL_PROCESS_ATTACH:
         DisableThreadLibraryCalls(hinstDLL);
         master_mutex = CreateMutexA( NULL, FALSE,
RPCSS_MASTER_MUTEX_NAME);
-        if (!master_mutex)
-          ERR("Failed to create master mutex\n");
         break;

case DLL_PROCESS_DETACH:

[Thomas Weidenmueller]

[e-mail] wrote:
> Remove bogus error message. It is normal for Mutex creation to fail
> since the Object will alredy exist after the first time it's created
> Modified: trunk/reactos/lib/rpcrt4/rpcrt4_main.c
>   _____
>
> Modified: trunk/reactos/lib/rpcrt4/rpcrt4_main.c
> --- trunk/reactos/lib/rpcrt4/rpcrt4_main.c	2005-04-20 12:46:39 UTC
> (rev 14710)
> +++ trunk/reactos/lib/rpcrt4/rpcrt4_main.c	2005-04-20 12:52:13 UTC
> (rev 14711)
> @@ -152,8 +152,6 @@
>  case DLL_PROCESS_ATTACH:
>      DisableThreadLibraryCalls(hinstDLL);
>      master_mutex = CreateMutexA( NULL, FALSE,
>
> RPCSS_MASTER_MUTEX_NAME);
>    if (!master_mutex)
>      ERR("Failed to create master mutex\n");
>    break;
>
> case DLL_PROCESS_DETACH:
>
> Ros-diffs mailing list
> [e-mail]
> http://reactos.com:8080/mailman/listinfo/ros-diffs

IMO you should open the mutex if creating it failed, otherwise 
master_mutex is NULL...

Best Regards,
Thomas

[Joseph Galbraith]

Thomas Weidenmueller wrote:
> [e-mail] wrote:
> > Remove bogus error message. It is normal for Mutex creation to fail
> > since the Object will alredy exist after the first time it's created
> > Modified: trunk/reactos/lib/rpcrt4/rpcrt4_main.c
> >   _____ 
> > Modified: trunk/reactos/lib/rpcrt4/rpcrt4_main.c
> > --- trunk/reactos/lib/rpcrt4/rpcrt4_main.c    2005-04-20 12:46:39 UTC
> > (rev 14710)
> > +++ trunk/reactos/lib/rpcrt4/rpcrt4_main.c    2005-04-20 12:52:13 UTC
> > (rev 14711)
> > @@ -152,8 +152,6 @@
> >  case DLL_PROCESS_ATTACH:
> >      DisableThreadLibraryCalls(hinstDLL);
> >      master_mutex = CreateMutexA( NULL, FALSE,
> >
> > RPCSS_MASTER_MUTEX_NAME);
> >    if (!master_mutex)
> >      ERR("Failed to create master mutex\n");
> >    break;
> >
> > case DLL_PROCESS_DETACH:
> >
> > Ros-diffs mailing list
> > [e-mail]
> > http://reactos.com:8080/mailman/listinfo/ros-diffs
>
> IMO you should open the mutex if creating it failed, otherwise 
> master_mutex is NULL...

I actually think the code as originally written was
correct.  (Although if ERR() is simply a debug tracing
macro, more error handling is probably needed.)

Regardless of whether the mutex is newly created or
pre-existing, CreateMutex() will return a handle to
it.

In the case of a pre-existing mutex, the handle to
the existing object is returned.

If it is important to know whether the mutex was
created or not, GetLastError() can be checked in
the success case for the value ERROR_ALREADY_EXISTS
(which in this case is not really an error, since the
function succeeded.)

If CreateMutex() returns NULL, it failed for some
other reason (than that the mutex already existed,
since CreateMutex doesn't fail in this case.)

Calling OpenMutex would not be productive in this case.

Thanks,

Joseph

[Thomas Weidenmueller]

Joseph Galbraith wrote:
> I actually think the code as originally written was
> correct.  (Although if ERR() is simply a debug tracing
> macro, more error handling is probably needed.)

No, this way only one application can successfully load it at the same time.
> Regardless of whether the mutex is newly created or
> pre-existing, CreateMutex() will return a handle to
> it.

CreateMutex fails if a named object is supposed to be created but 
already exists. GetLastError() then will - as you mentioned below - 
return ERROR_ALREADY_EXISTS or another failure code.
> In the case of a pre-existing mutex, the handle to
> the existing object is returned.

Not by CreateMutex.
> If it is important to know whether the mutex was
> created or not, GetLastError() can be checked in
> the success case for the value ERROR_ALREADY_EXISTS
> (which in this case is not really an error, since the
> function succeeded.)

The last error code won't be changed by CreateMutex if it returns 
success. There's only a few APIs that do that.
> If CreateMutex() returns NULL, it failed for some
> other reason (than that the mutex already existed,
> since CreateMutex doesn't fail in this case.)

Quote from the PSDK: "If lpName matches the name of an existing event, 
semaphore, waitable timer, job, or file-mapping object, the function 
fails and the GetLastError function returns ERROR_INVALID_HANDLE. This 
occurs because these objects share the same name space."
> Calling OpenMutex would not be productive in this case.

It is the only possibility.

Best Regards,
Thomas

[Hartmut Birr]

Thomas Weidenmueller wrote:
> Joseph Galbraith wrote:
> > Regardless of whether the mutex is newly created or
> > pre-existing, CreateMutex() will return a handle to
> > it.
>
> CreateMutex fails if a named object is supposed to be created but
> already exists. GetLastError() then will - as you mentioned below -
> return ERROR_ALREADY_EXISTS or another failure code.

This is not correct. CreateMutex fails only if the named object already
exists and if the object is not a mutex.
> > In the case of a pre-existing mutex, the handle to
> > the existing object is returned.
>
> Not by CreateMutex.

This is not correct. If the mutex already exist, CreateMutex returns a
handle to the existing mutex.
> > If CreateMutex() returns NULL, it failed for some
> > other reason (than that the mutex already existed,
> > since CreateMutex doesn't fail in this case.)
>
> Quote from the PSDK: "If lpName matches the name of an existing event,
> semaphore, waitable timer, job, or file-mapping object, the function
> fails and the GetLastError function returns ERROR_INVALID_HANDLE. This
> occurs because these objects share the same name space."

Some lines later in the PSDK: "If the function succeeds, the return
value is a handle to the mutex object. If the named mutex object existed
before the function call, the function returns a handle to the existing
object and GetLastError() returns ERROR_ALREADY_EXISTS. Otherwise, the
caller created the mutex."

- Hartmut

[Joseph Galbraith]

Thomas Weidenmueller wrote:
> Joseph Galbraith wrote:
> > If CreateMutex() returns NULL, it failed for some
> > other reason (than that the mutex already existed,
> > since CreateMutex doesn't fail in this case.)
>
> Quote from the PSDK: "If lpName matches the name of an existing event, 
> semaphore, waitable timer, job, or file-mapping object, the function 
> fails and the GetLastError function returns ERROR_INVALID_HANDLE. This 
> occurs because these objects share the same name space."

Ahh... I see the confusion;  I'll counter with another quote
from the platform SDK:

Return Values

If the function succeeds, the return value is a handle
   to the mutex object. If the named mutex object existed
   before the function call, the function returns a handle
   to the existing object and GetLastError returns
   ERROR_ALREADY_EXISTS. Otherwise, the caller created the mutex.

If the function fails, the return value is NULL. To get
   extended error information, call GetLastError.

If you read your quote carefully, you'll notice that it is
refering to existing events, semaphores, waitable timers, jobs,
or file-mapping objets... mutex is notably absent from the list.
The key is the last sentance:  "This occurs because these objects
[the ones listed] share the same name space [as mutex objects.]

I do see now where the confusion comes from though; at first I was
just going to respond that the two pieces of platform SDK documentation
were in conflict, and the one I quoted is the behavior observed
experimentally... then I read carefully.

Thanks,

Joseph

[Thomas Weidenmueller]

Joseph Galbraith wrote:
> Ahh... I see the confusion;  I'll counter with another quote
> from the platform SDK:
>
> Return Values
>
> If the function succeeds, the return value is a handle
>   to the mutex object. If the named mutex object existed
>   before the function call, the function returns a handle
>   to the existing object and GetLastError returns
>   ERROR_ALREADY_EXISTS. Otherwise, the caller created the mutex.
>
> If the function fails, the return value is NULL. To get
>   extended error information, call GetLastError.
>
> If you read your quote carefully, you'll notice that it is
> refering to existing events, semaphores, waitable timers, jobs,
> or file-mapping objets... mutex is notably absent from the list.
> The key is the last sentance:  "This occurs because these objects
> [the ones listed] share the same name space [as mutex objects.]
>
> I do see now where the confusion comes from though; at first I was
> just going to respond that the two pieces of platform SDK documentation
> were in conflict, and the one I quoted is the behavior observed
> experimentally... then I read carefully.

Thanks for clearing this up, I didn't read the entire documentation, 
blame me. Maybe we should inform the MSDN team.

Best Regards,
Thomas

[Joseph Galbraith]

Thomas Weidenmueller wrote:
> Joseph Galbraith wrote:
> > Ahh... I see the confusion;  I'll counter with another quote
> > from the platform SDK:
> >
> > Return Values
> >
> > If the function succeeds, the return value is a handle
> >   to the mutex object. If the named mutex object existed
> >   before the function call, the function returns a handle
> >   to the existing object and GetLastError returns
> >   ERROR_ALREADY_EXISTS. Otherwise, the caller created the mutex.
> >
> > If the function fails, the return value is NULL. To get
> >   extended error information, call GetLastError.
> >
> > If you read your quote carefully, you'll notice that it is
> > refering to existing events, semaphores, waitable timers, jobs,
> > or file-mapping objets... mutex is notably absent from the list.
> > The key is the last sentance:  "This occurs because these objects
> > [the ones listed] share the same name space [as mutex objects.]
> >
> > I do see now where the confusion comes from though; at first I was
> > just going to respond that the two pieces of platform SDK documentation
> > were in conflict, and the one I quoted is the behavior observed
> > experimentally... then I read carefully.
>
> Thanks for clearing this up, I didn't read the entire documentation, 
> blame me. Maybe we should inform the MSDN team.

Sure, I'll report a PSDK documentation bug.

Thanks,

Joseph