2023-10-16 20:42:37

by enh

[permalink] [raw]
Subject: [PATCH] scsi_status_is_good() uses __KERNEL__ constants.

Without this missing #ifdef, userspace code trying to use this header
directly won't compile. glibc manually removes it, bionic removes it
using a script. If we add this, the preprocessor can remove it instead.

Signed-off-by: Elliott Hughes <[email protected]>
---
include/scsi/scsi.h | 2 ++
1 file changed, 2 insertions(+)

diff --git a/include/scsi/scsi.h b/include/scsi/scsi.h
index ec093594ba53..a585e2067373 100644
--- a/include/scsi/scsi.h
+++ b/include/scsi/scsi.h
@@ -186,6 +186,7 @@ enum scsi_disposition {
/* Used to obtain the PCI location of a device */
#define SCSI_IOCTL_GET_PCI 0x5387

+#ifdef __KERNEL__
/** scsi_status_is_good - check the status return.
*
* @status: the status passed up from the driver (including host and
@@ -216,5 +217,6 @@ static inline bool scsi_status_is_good(int status)
/* FIXME: this is obsolete in SAM-3 */
(status == SAM_STAT_COMMAND_TERMINATED));
}
+#endif /* __KERNEL__ */

#endif /* _SCSI_SCSI_H */
--
2.42.0.655.g421f12c284-goog


2023-10-16 20:49:09

by Bart Van Assche

[permalink] [raw]
Subject: Re: [PATCH] scsi_status_is_good() uses __KERNEL__ constants.

On 10/16/23 13:42, enh wrote:
> Without this missing #ifdef, userspace code trying to use this header
> directly won't compile. glibc manually removes it, bionic removes it
> using a script. If we add this, the preprocessor can remove it instead.

Is that the right solution? Shouldn't these software projects be
modified such that <scsi/scsi.h> is *not* included?

Thanks,

Bart.

2023-10-16 22:06:57

by enh

[permalink] [raw]
Subject: Re: [PATCH] scsi_status_is_good() uses __KERNEL__ constants.

On Mon, Oct 16, 2023 at 1:48 PM Bart Van Assche <[email protected]> wrote:
>
> On 10/16/23 13:42, enh wrote:
> > Without this missing #ifdef, userspace code trying to use this header
> > directly won't compile. glibc manually removes it, bionic removes it
> > using a script. If we add this, the preprocessor can remove it instead.
>
> Is that the right solution? Shouldn't these software projects be
> modified such that <scsi/scsi.h> is *not* included?

i'm not sure that's practical? all linux libcs i know of include these
headers. (though bionic only has them because glibc did. i'm assuming
the same is true for musl?)

i think there's obviously a question of "why aren't these uapi
headers, if stuff is using them?". just looking at Android, i see
sg3_utils, mtools, compiler-rt (for ioctls), and toybox (for the eject
command).

or perhaps --- even if most of the scsi headers should be non-uapi, is
there a subset of stuff that should be in uapi?

but "libc can use this header directly like it does uapi headers,
rather than having to need a human manually fix the header" seemed
like a step forward from the status quo where everyone's shipping
their own hacked-up versions of these headers?

> Thanks,
>
> Bart.
>