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> 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> > 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
_______________________________________________ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel