| Message ID | 20201216172719.540610-1-luca.boccassi@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [fsverity-utils,1/2] Remove unneeded includes | expand |
On Wed, Dec 16, 2020 at 05:27:18PM +0000, luca.boccassi@gmail.com wrote: > From: Luca Boccassi <luca.boccassi@microsoft.com> > > Signed-off-by: Luca Boccassi <luca.boccassi@microsoft.com> > --- > common/fsverity_uapi.h | 1 - > programs/cmd_enable.c | 1 - > 2 files changed, 2 deletions(-) > > diff --git a/common/fsverity_uapi.h b/common/fsverity_uapi.h > index 33f4415..0006c35 100644 > --- a/common/fsverity_uapi.h > +++ b/common/fsverity_uapi.h > @@ -10,7 +10,6 @@ > #ifndef _UAPI_LINUX_FSVERITY_H > #define _UAPI_LINUX_FSVERITY_H > > -#include <linux/ioctl.h> > #include <linux/types.h> fsverity_uapi.h is supposed to be kept in sync with include/uapi/linux/fsverity.h in the kernel source tree. There's a reason why it includes <linux/ioctl.h>. <linux/ioctl.h> provides the _IOW() and _IOWR() macros to expand the values of FS_IOC_ENABLE_VERITY and FS_IOC_MEASURE_VERITY. Usually people referring to FS_IOC_* will include <sys/ioctl.h> in order to actually call ioctl() too. However it's not guaranteed, so technically the header needs to include <linux/ioctl.h>. Wrapping this include with '#ifdef _WIN32' would be better than removing it, as then it would be clear that it's a Windows-specific modification. However I think an even better approach would be to add empty files win32-headers/linux/{types,ioctl.h} and add -Iwin32-headers to CPPFLAGS, so that these headers can still be included in the Windows build without having to modify the source files. > diff --git a/programs/cmd_enable.c b/programs/cmd_enable.c > index fdf26c7..14c3c17 100644 > --- a/programs/cmd_enable.c > +++ b/programs/cmd_enable.c > @@ -14,7 +14,6 @@ > #include <fcntl.h> > #include <getopt.h> > #include <limits.h> > -#include <sys/ioctl.h> > This part looks fine though, as cmd_enable.c no longer directly uses an ioctl. - Eric
On Wed, 2020-12-16 at 10:44 -0800, Eric Biggers wrote: > On Wed, Dec 16, 2020 at 05:27:18PM +0000, luca.boccassi@gmail.com wrote: > > From: Luca Boccassi <luca.boccassi@microsoft.com> > > > > Signed-off-by: Luca Boccassi <luca.boccassi@microsoft.com> > > --- > > common/fsverity_uapi.h | 1 - > > programs/cmd_enable.c | 1 - > > 2 files changed, 2 deletions(-) > > > > diff --git a/common/fsverity_uapi.h b/common/fsverity_uapi.h > > index 33f4415..0006c35 100644 > > --- a/common/fsverity_uapi.h > > +++ b/common/fsverity_uapi.h > > @@ -10,7 +10,6 @@ > > #ifndef _UAPI_LINUX_FSVERITY_H > > #define _UAPI_LINUX_FSVERITY_H > > > > -#include <linux/ioctl.h> > > #include <linux/types.h> > > fsverity_uapi.h is supposed to be kept in sync with > include/uapi/linux/fsverity.h in the kernel source tree. > > There's a reason why it includes <linux/ioctl.h>. <linux/ioctl.h> provides the > _IOW() and _IOWR() macros to expand the values of FS_IOC_ENABLE_VERITY and > FS_IOC_MEASURE_VERITY. Usually people referring to FS_IOC_* will include > <sys/ioctl.h> in order to actually call ioctl() too. However it's not > guaranteed, so technically the header needs to include <linux/ioctl.h>. > > Wrapping this include with '#ifdef _WIN32' would be better than removing it, as > then it would be clear that it's a Windows-specific modification. > > However I think an even better approach would be to add empty files > win32-headers/linux/{types,ioctl.h} and add -Iwin32-headers to CPPFLAGS, so that > these headers can still be included in the Windows build without having to > modify the source files. I took the first approach in v2, as it's a bit simpler. If you feel strongly about it I can do the latter. > > diff --git a/programs/cmd_enable.c b/programs/cmd_enable.c > > index fdf26c7..14c3c17 100644 > > --- a/programs/cmd_enable.c > > +++ b/programs/cmd_enable.c > > @@ -14,7 +14,6 @@ > > #include <fcntl.h> > > #include <getopt.h> > > #include <limits.h> > > -#include <sys/ioctl.h> > > > > This part looks fine though, as cmd_enable.c no longer directly uses an ioctl. > > - Eric
diff --git a/common/fsverity_uapi.h b/common/fsverity_uapi.h index 33f4415..0006c35 100644 --- a/common/fsverity_uapi.h +++ b/common/fsverity_uapi.h @@ -10,7 +10,6 @@ #ifndef _UAPI_LINUX_FSVERITY_H #define _UAPI_LINUX_FSVERITY_H -#include <linux/ioctl.h> #include <linux/types.h> #define FS_VERITY_HASH_ALG_SHA256 1 diff --git a/programs/cmd_enable.c b/programs/cmd_enable.c index fdf26c7..14c3c17 100644 --- a/programs/cmd_enable.c +++ b/programs/cmd_enable.c @@ -14,7 +14,6 @@ #include <fcntl.h> #include <getopt.h> #include <limits.h> -#include <sys/ioctl.h> static bool read_signature(const char *filename, u8 **sig_ret, u32 *sig_size_ret)