RTEMS | Draft: cpukit/posix/aio*: Create notification at request completion (!118)

[Joel Sherrill (@joel)]

Merge request https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/118 was reviewed by Joel Sherrill

--
  
Joel Sherrill started a new discussion on cpukit/include/rtems/posix/aio_misc.h: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/118#note_109373

 > +/**
 > + * @brief Generates a notification based on the sigenvent sructure passed as parameter.
 > + * 

Shorten the brief. Think section heading in Table of Contents.

Turn the details into a first paragraph.

Add @param for the argument.

--
  
Joel Sherrill started a new discussion on cpukit/posix/src/aio_misc.c: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/118#note_109374

 >   */
 >  
 > +

Remove extra blank line.

--
  
Joel Sherrill started a new discussion on cpukit/posix/src/aio_misc.c: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/118#note_109375

 > +
 >  #include <pthread.h>
 > +#include <stdio.h>

Is this for your debugging or needed permanently?

--
  
Joel Sherrill started a new discussion on cpukit/posix/src/aio_misc.c: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/118#note_109376

 > +        sig->sigev_value
 > +      );
 > +      if ( result != 0 ){

Space needed. "){" should be ") {".

I see this multiple places. Search and destroy.

--
  
Joel Sherrill started a new discussion on cpukit/posix/src/aio_misc.c: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/118#note_109377

 > +
 > +      if ( sig->sigev_notify_function == NULL ){
 > +        //handle error

There isn't really anyway to handle an error here. Should this be caught at the API level so an error can be returned to the caller?

If so, then the checks which were in here can become debug asserts.

--
  
Joel Sherrill started a new discussion on cpukit/posix/src/aio_misc.c: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/118#note_109378

 > +        &thread,
 > +        attr,
 > +        (void * (*)(void *)) sig->sigev_notify_function,

You need to wrap the sigev_notification function in a wrapper thread body which has the proper signature. Pass in the notify function as the argument, cast it, call it, and return NULL. It may also make sense to explicitly call pthread_exit() at the bottom of the wrapper function.

--
  
Joel Sherrill started a new discussion on cpukit/posix/src/aio_misc.c: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/118#note_109379

 >    }
 > +  rtems_aio_notify(
 > +    &(req->aiocbp->aio_sigevent)

Parentheses are not needed.

Can fit on one line.

--
  
Joel Sherrill started a new discussion on cpukit/posix/src/aio_misc.c: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/118#note_109380

 >        result = -1;
 >    }
 > +  rtems_aio_notify(

Looks like there needs to be a blank line above this.

--
  
Joel Sherrill started a new discussion on spec/build/testsuites/psxtests/psxaio04.yml: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/118#note_109381

 > +cflags: []
 > +copyrights:
 > +- Copyright (C) 2020 embedded brains GmbH & Co. KG

You can put your name and copyright on any file of your own creation or that you have modified more than a few lines.

--
  
Joel Sherrill started a new discussion on testsuites/psxtests/psxaio04/init.c: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/118#note_109382

 > +}
 > +
 > +

Remove a blank line here.

--
  
Joel Sherrill started a new discussion on testsuites/psxtests/psxaio04/init.c: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/118#note_109383

 > +
 > +/*
 > + * Copyright 2010, Alin Rus <alin.codejunkie at gmail.com>

Is this completely new? Modified? Does your copyright and 2024 need to be added?

--
  
Joel Sherrill started a new discussion on testsuites/psxtests/psxaio04/init.c: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/118#note_109384

 > +  rtems_test_assert( !status );
 > +  
 > +  fd = open( "/tmp/aio_fildes", O_RDWR|O_CREAT, S_IRWXU|S_IRWXG|S_IRWXO );

Is this line too long?

--
  
Joel Sherrill started a new discussion on testsuites/psxtests/psxaio04/system.h: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/118#note_109385

 > +
 > +/*
 > + * Copyright 2010, Alin Rus <alin.codejunkie at gmail.com>

Again check need for your copyright




-- 
View it on GitLab: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/118
You're receiving this email because of your account on gitlab.rtems.org.


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.rtems.org/pipermail/bugs/attachments/20240716/f572eecf/attachment-0001.htm>