This needs a ticket now to apply to 5, and we'll want to apply to 6 also.
Can you open a ticket and post a patch for 5.1?
On Thu, Jun 25, 2020, 5:49 PM Kinsey Moore <kinsey.mo...@oarcorp.com> wrote:
> Hey Gedare,
> Setting obj_err to 0 would get the desired outcome, but the logic as it
> exists now is faulty. Setting obj_err to a value that can't be produced by
> _POSIX_Shm_Get_by_name would be a better option, but it would still be
> checking an error value after successful execution. _POSIX_Shm_Get_by_name
> relies on _Objects_Get_by_name internally which states about obj_err: The
> error indication in case of failure. If _POSIX_Shm_Get_by_name returns
> OBJECTS_GET_BY_NAME_NO_OBJECT (which it can), the current logic treats that
> as a success and operates on a NULL pointer.
>
> -----Original Message-----
> From: Gedare Bloom <ged...@rtems.org>
> Sent: Thursday, June 25, 2020 16:49
> To: Kinsey Moore <kinsey.mo...@oarcorp.com>
> Cc: devel@rtems.org
> Subject: Re: [PATCH] posix: Only check shm_unlink obj_err if necessary
>
> Hi Kinsey,
>
> I missed seeing this. Two quick questions for you.
>
> 1. does it also work to initialize obj_err to 0? that would be simpler
> code.
>
> 2. I see the error handling logic changes slightly, with
> OBJECTS_GET_BY_NAME_NO_OBJECT now returning ENOENT. I guess if it works to
> init obj_err to 0, this case should be merged back to the
> OBJECTS_GET_BY_NAME_INVALID_NAME case?
>
> On Thu, Jun 25, 2020 at 2:21 PM Kinsey Moore <kinsey.mo...@oarcorp.com>
> wrote:
> >
> > Is there anything stopping this from being merged? I just ran into this
> bug again on the current project I'm working on.
> >
> > Kinsey
> >
> > -----Original Message-----
> > From: Kinsey Moore <kinsey.mo...@oarcorp.com>
> > Sent: Tuesday, January 28, 2020 12:37
> > To: devel@rtems.org
> > Cc: Kinsey Moore <kinsey.mo...@oarcorp.com>
> > Subject: [PATCH] posix: Only check shm_unlink obj_err if necessary
> >
> > In the nominal case checked by spsysinit01, obj_err is unmodified if
> _POSIX_Shm_Get_by_name returns non-NULL. In the case of shm_unlink, this
> means an uninitialized value is passed into the switch and it appears this
> test was passing by virtue of the stack having the right value on it in
> most cases. This now checks obj_err only if _POSIX_Shm_Get_by_name returns
> NULL.
> > ---
> > cpukit/posix/src/shmunlink.c | 45
> > ++++++++++++++++++------------------
> > 1 file changed, 23 insertions(+), 22 deletions(-)
> >
> > diff --git a/cpukit/posix/src/shmunlink.c
> > b/cpukit/posix/src/shmunlink.c index 00d743ac80..39c2ba0d87 100644
> > --- a/cpukit/posix/src/shmunlink.c
> > +++ b/cpukit/posix/src/shmunlink.c
> > @@ -29,28 +29,29 @@ int shm_unlink( const char *name )
> > _Objects_Allocator_lock();
> >
> > shm = _POSIX_Shm_Get_by_name( name, 0, &obj_err );
> > - switch ( obj_err ) {
> > - case OBJECTS_GET_BY_NAME_INVALID_NAME:
> > - err = ENOENT;
> > - break;
> > -
> > - case OBJECTS_GET_BY_NAME_NAME_TOO_LONG:
> > - err = ENAMETOOLONG;
> > - break;
> > -
> > - case OBJECTS_GET_BY_NAME_NO_OBJECT:
> > - default:
> > - _Objects_Namespace_remove_string(
> > - &_POSIX_Shm_Information,
> > - &shm->Object
> > - );
> > -
> > - if ( shm->reference_count == 0 ) {
> > - /* Only remove the shm object if no references exist to it.
> Otherwise,
> > - * the shm object will be freed later in
> _POSIX_Shm_Attempt_delete */
> > - _POSIX_Shm_Free( shm );
> > - }
> > - break;
> > + if ( shm ) {
> > + _Objects_Namespace_remove_string(
> > + &_POSIX_Shm_Information,
> > + &shm->Object
> > + );
> > +
> > + if ( shm->reference_count == 0 ) {
> > + /* Only remove the shm object if no references exist to it.
> Otherwise,
> > + * the shm object will be freed later in
> _POSIX_Shm_Attempt_delete */
> > + _POSIX_Shm_Free( shm );
> > + }
> > + } else {
> > + switch ( obj_err ) {
> > + case OBJECTS_GET_BY_NAME_NAME_TOO_LONG:
> > + err = ENAMETOOLONG;
> > + break;
> > +
> > + case OBJECTS_GET_BY_NAME_INVALID_NAME:
> > + case OBJECTS_GET_BY_NAME_NO_OBJECT:
> > + default:
> > + err = ENOENT;
> > + break;
> > + }
> > }
> >
> > _Objects_Allocator_unlock();
> > --
> > 2.20.1
> >
> > _______________________________________________
> > devel mailing list
> > devel@rtems.org
> > http://lists.rtems.org/mailman/listinfo/devel
>
_______________________________________________
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel
