Sorry, accidentally sent from my personal account. That was, in fact, me. Kinsey
From: Will <nyphb...@gmail.com> Sent: Saturday, June 27, 2020 19:24 To: Gedare Bloom <ged...@rtems.org> Cc: Kinsey Moore <kinsey.mo...@oarcorp.com>; devel@rtems.org Subject: Re: [PATCH] posix: Only check shm_unlink obj_err if necessary Ticket 4016 opened for 5.1 and patch sent with appropriate close tag. On Sat, Jun 27, 2020 at 4:30 PM Gedare Bloom <ged...@rtems.org<mailto:ged...@rtems.org>> wrote: 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<mailto: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<mailto:ged...@rtems.org>> Sent: Thursday, June 25, 2020 16:49 To: Kinsey Moore <kinsey.mo...@oarcorp.com<mailto:kinsey.mo...@oarcorp.com>> Cc: devel@rtems.org<mailto: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<mailto: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<mailto:kinsey.mo...@oarcorp.com>> > Sent: Tuesday, January 28, 2020 12:37 > To: devel@rtems.org<mailto:devel@rtems.org> > Cc: Kinsey Moore <kinsey.mo...@oarcorp.com<mailto: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<mailto:devel@rtems.org> > http://lists.rtems.org/mailman/listinfo/devel _______________________________________________ devel mailing list devel@rtems.org<mailto:devel@rtems.org> http://lists.rtems.org/mailman/listinfo/devel
_______________________________________________ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel
