NVIDIA / pyxis

Container plugin for Slurm Workload Manager
Apache License 2.0
273 stars 31 forks source link

Overload the --container-image option to allow squashfs path #11

Closed lukeyeager closed 4 years ago

flx42 commented 4 years ago

Thanks for the fix. I'm not a big fan of putting the PATH_MAX check here, as it is related to a local variable is declared in another function. And the compiler returns a warning as the usage of strncpy is still suspicious:

In function ‘strncpy’,
    inlined from ‘enroot_container_create.isra.0’ at pyxis_slurmstepd.c:865:3:
/usr/include/x86_64-linux-gnu/bits/string_fortified.h:106:10: warning: ‘__builtin_strncpy’ specified bound 4096 equals destination size [-Wstringop-truncation]
  106 |   return __builtin___strncpy_chk (__dest, __src, __len, __bos (__dest));
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

What I had in mind was building the URI in the spank_option_image function, but I realize now that it won't really simplify the code. So I'll put the strnlen in enroot_container_create, along with forcing a null terminator for strncpy.

flx42 commented 4 years ago

Modified and merged manually as 84a31ce4260cd8495e60f7d3eaa5d11fa06ef495, thanks again!