Re: [PATCH 4/9] pppd: include time.h before using time_t
On 10/04/19 10:29, Kurt Van Dijck wrote: > I don't know a system where (a) or (b) are valid. My point is that such > system could could exist, so I learned not to inspect the header files > looking for a type, but inspect man-pages or specifications when looking > for a type, and so time_t is defined in time.h. I didn't just go trolling a grepping for time_t. sys/time.h is pretty well-established in UNIX, and I think you're punting when you point to ANSI C alone as the authority here. As for documentation, how does SUSv2 seem? https://pubs.opengroup.org/onlinepubs/7908799/xsh/systime.h.html > Now that I know that that file is used as include for kernel code, I'd > rather include time.h in the userspace c-files. My point is that include/net/ isn't strictly userspace. If you feel the need, then go ahead and include in user level files. This just isn't one of those. If you must do this in ppp_def.h, then it needs to be guarded against *all* of the systems where including a top-level header file inside a kernel module is the wrong thing to do, not just "ifndef SOLARIS". Do you know which systems those are? I can tell you that Solaris/Illumos is at least one such system, but I can't tell you that it's *all* of them. I think this include is out of place here. -- James Carlson 42.703N 71.076W
Re: [PATCH 4/9] pppd: include time.h before using time_t
On vr, 04 okt 2019 08:52:12 -0400, James Carlson wrote: > On 10/04/19 06:49, Kurt Van Dijck wrote: > >> IMHO time_t is defined in sys/types.h > > > > http://www.open-std.org/JTC1/SC22/WG14/www/docs/n1256.pdf > > chapter 7.23.1.3 > > > > I believe that covers userland environments, not the kernel. > > At least on Solaris (and its derivatives, such as Illumos), the symbols > available in the kernel are defined in sys/ (or net/, netinet/, or > similar for network bits). The top-level header files are for userland > libraries. Userland libraries are not accessible within the kernel. > > In this case, the common net/ppp_defs.h file is used by both user-level > code (pppd itself) and by several kernel modules. I see. > > There may be systems on which including within a kernel module > is harmless (I suspect Linux is one), but I have a hard time believing > that it's correct to do so. You're right that the kernel code does not __necessarily__ use the same thing. What matters here is that all kernel code must use the same thing. > > Do you know of a system where either (a) does not exist or > (b) it exists but does not define 'time_t'? I haven't been able to find > a system that matches either case. I tried several flavors of Linux, > AIX, Solaris, HP/UX, and IBM USS on z/OS. I don't know a system where (a) or (b) are valid. My point is that such system could could exist, so I learned not to inspect the header files looking for a type, but inspect man-pages or specifications when looking for a type, and so time_t is defined in time.h. Regardless of those systems, you look for 1 header that suits userspace and solaris kernel. Isn't that a bit ... strange. Now that I know that that file is used as include for kernel code, I'd rather include time.h in the userspace c-files. Kurt
Re: [PATCH 4/9] pppd: include time.h before using time_t
> IMHO time_t is defined in sys/types.h http://www.open-std.org/JTC1/SC22/WG14/www/docs/n1256.pdf chapter 7.23.1.3
Re: [PATCH 4/9] pppd: include time.h before using time_t
IMHO time_t is defined in sys/types.h Lev On Fri, Oct 4, 2019 at 9:28 AM Paul Mackerras wrote: > > On Thu, Sep 26, 2019 at 09:21:01AM +0200, Kurt Van Dijck wrote: > > Signed-off-by: Kurt Van Dijck > > --- > > include/net/ppp_defs.h | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/include/net/ppp_defs.h b/include/net/ppp_defs.h > > index b06eda5..ed04486 100644 > > --- a/include/net/ppp_defs.h > > +++ b/include/net/ppp_defs.h > > @@ -35,6 +35,8 @@ > > * OUT OF OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. > > */ > > > > +#include > > + > > #ifndef _PPP_DEFS_H_ > > #define _PPP_DEFS_H_ > > I applied this series, but then reverted this one because it breaks > compilation of the kernel device driver on Solaris. What exactly is > the error that you are seeing without this #include? Would your error > be fixed by including (which would be OK on Solaris)? > > Paul.