[PATCH rtems-libbsd] ipsec-tools: Fix copying fd_set prior to select

[Chris Johns]

On 23/5/2023 4:25 pm, Christian MAUDERER wrote:
> Hello Chris,
> 
> On 2023-05-23 03:36, Chris Johns wrote:
>> Hi,
>>
>> I have been resolving this by adding:
>>
>> #define preset_mask *preset_mask_prealloc
>> #define active_mask *active_mask_prealloc
>>
>> as the assignment logic works as is. It does not catch FD_ZERO which is a shame
>> but is helps avoid us adding a special case.
> 
> Thanks for the suggestion. I used that solution already in the file during the
> initial port. But it doesn't work in this case (and maybe we should check other
> cases in libbsd too). It only copies the first element of the array of fd_sets.
> 
> The variables / defines are (in the patched version - see [1]):
> 
> static fd_set *allocated_preset_mask, *allocated_active_mask;
> static struct fd_monitor *allocated_fd_monitors;
> #define preset_mask (*allocated_preset_mask)
> #define active_mask (*allocated_active_mask)
> #define fd_monitors (allocated_fd_monitors)
> 
> They are allocated as follows (see [2]):
> 
>     allocated_preset_mask = calloc(sizeof(fd_set),
>         howmany(rtems_libio_number_iops, sizeof(fd_set) * 8));
>     allocated_active_mask = calloc(sizeof(fd_set),
>         howmany(rtems_libio_number_iops, sizeof(fd_set) * 8));
>     allocated_fd_monitors = calloc(
>         rtems_libio_number_iops, sizeof(struct fd_monitor));
> 
> So basically it's a bunch of arrays of fd_sets. If I now use the following
> (unpatched) assignment:
> 
>     active_mask = preset_mask;
> 
> After the preprocessor the line will be
> 
>     (*allocated_active_mask) = (*allocated_preset_mask);
> 
> Which copies only the first element. That's why I had to add the memcpy. Did I
> miss some better solution?

I am using this in ntpd which I checked yesterday:

#ifdef __rtems__
#include <rtems/libio_.h>
static size_t rtems_ntpd_fds_size;
static int rtems_fd_set_alloc(fd_set **setp) {
        if (*setp == NULL) {
                rtems_ntpd_fds_size = sizeof(fd_set) *
(howmany(rtems_libio_number_iops, sizeof(fd_set) * 8));
                *setp = malloc(rtems_ntpd_fds_size);
                if (*setp == NULL) {
                        errno = ENOMEM;
                        return -1;
                }
                memset(*setp, 0, rtems_ntpd_fds_size);
        }
        return 0;
}
#define activefds (*activefds_prealloc)
static fd_set *activefds_prealloc;
#define rtems_activefds_alloc() rtems_fd_set_alloc(&activefds_prealloc)
#else /* __rtems__ */
static fd_set activefds;
#endif /* __rtems__ */

and:

#if __rtems__
        #define rdfdes (*rdfdes_prealloc)
        static fd_set *rdfdes_prealloc;
#else /* __rtems__ */
        fd_set rdfdes;
#endif /* __rtems__ */
        int nfound;

#if __rtems__
        if (rtems_fd_set_alloc(&rdfdes_prealloc) < 0) {
                return;
        }
        memset(rdfdes_prealloc, 0, rtems_ntpd_fds_size);
#else /* __rtems__ */
        FD_ZERO(&set);
#endif /* __rtems__ */

with:

        ++handler_calls;
        rdfdes = activefds;

Stepping:

Thread 9 hit Breakpoint 1, _ntp_io_handler () at
../../bsd/freebsd/contrib/ntp/ntpd/ntp_io.c:3727
3727            rdfdes = activefds;
(gdb) x /8bx activefds_prealloc
0x1083e650:     0x20    0x44    0x05    0x20    0x00    0x40    0x00    0x00
(gdb) x /8bx rdfdes_prealloc
0x108353a0:     0x00    0x00    0x00    0x00    0x00    0x00    0x00    0x00
(gdb) n
3736                    t1.tv_usec = 0;
(gdb) x /8bx rdfdes_prealloc
0x108353a0:     0x20    0x44    0x05    0x20    0x00    0x40    0x00    0x00

Looks ok to me.

Chris