Add support for O_DSYNC in aio_fsync()

The second issue I addressed was a lack of functionality in the aio_fsync() function. According to the specifications, the op parameter of the function can have two values: O_SYNC and O_DSYNC. This value indicates what kind of synchronization has to be performed on the file. Specifically, O_SYNC will sync the file as per a call to fsync(), while O_DSYNC corresponds to a call to fdatasync().

Before beginning the work on this part, I encountered a blocking issue. Specifically, O_DSYNC was not defined. I discovered this while looking for existing issues related to my project. The issue I found was issue #1683, which highlighted the missing definitions of O_DSYNC and O_RSYNC in Newlib. After verifying that the issue was not outdated and discussing the problem with my mentors, we reached the following solution: To allow me to begin the work on that part, I would temporarily define the missing macros in the installed files of Newlib on my machine. While I did this, my mentor submitted a patch to Newlib that added the missing macros. This way, I managed to begin writing a first version of the fix.

Adding this functionality did not require much code. The main change was in the body of the thread(s) servicing the requests. In the code, there’s a switch statement that identifies the kind of operation; I had to add another case for the new sync operation. I also had to modify the aio_fsync() function to allow the use of the O_DSYNC value.

After the update to Newlib, I opened a Merge Request (MR!128) to get feedback on the code. The review process highlighted an issue that needed to be fixed:

The implementation used some opcodes to identify the various types of operations. The codes used were those defined in the aio.h header and required by lio_listio(). However, since the specification only includes opcodes for the read and write operations (LIO_READ and LIO_WRITE), some additional ones were defined in the aio.h header. Since these additional opcodes were only for internal use, and the others were intended to be used with lio_listio(), under the suggestion of my mentor, I deleted the additional codes from aio.h and defined private opcodes for each operation. I then modified the various functions, making all the code use the new private opcodes to identify the various operations internally.

After this and other smaller formatting corrections, the code got merged. After the merge, however, Coverity highlighted an issue in the newly added code. It marked a branch of an if statement as dead code, with the following message:

New defect(s) Reported-by: Coverity Scan
Showing 1 of 1 defect(s)


** CID 1616018:  Control flow issues  (DEADCODE)
/cpukit/posix/src/aio_fsync.c: 83 in aio_fsync()


________________________________________________________________________________________________________
*** CID 1616018:  Control flow issues  (DEADCODE)
/cpukit/posix/src/aio_fsync.c: 83 in aio_fsync()
77         rtems_set_errno_and_return_minus_one( EAGAIN );
78     
79       req->aiocbp = aiocbp;
80       if ( op == O_SYNC ) {
81         req->op_type = AIO_OP_SYNC;
82       } else {
>>>     CID 1616018:  Control flow issues  (DEADCODE)
>>>     Execution cannot reach this statement: "req->op_type = 3;".
83         req->op_type = AIO_OP_DSYNC;
84       }
85       
86       return rtems_aio_enqueue( req );

I tried to understand the reason for this message but was unsuccessful. After some investigation, my mentors discovered the reason for this error. Both O_SYNC and O_DSYNC are mapped to the same value, and since O_SYNC and O_DSYNC are the only two values permitted for op (this condition was checked by an earlier line in the function), when execution reached line 80, the condition was always true.

To solve this problem, we ended up reverting some of the changes made, removing the handling of O_DSYNC from aio_fsync(). This does not actually remove support for O_DSYNC, but since both constants have the same value, O_DSYNC will be treated as O_SYNC. This solution only works if O_DSYNC==O_SYNC. To be notified if that changes, we added the following code to the aio_fsync.c file:

#if (O_DSYNC != O_SYNC)
  #error "Implementation assumes O_DSYNC == O_SYNC. Update if this changes."
#endif 

In this way, we will know if the values change and can reintroduce support for both values.