From owner-svn-src-head@freebsd.org Thu Jun 20 17:22:05 2019 Return-Path: Delivered-To: svn-src-head@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id A2EF315C230B; Thu, 20 Jun 2019 17:22:05 +0000 (UTC) (envelope-from asomers@gmail.com) Received: from mail-lj1-f176.google.com (mail-lj1-f176.google.com [209.85.208.176]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) server-signature RSA-PSS (4096 bits) client-signature RSA-PSS (2048 bits) client-digest SHA256) (Client CN "smtp.gmail.com", Issuer "GTS CA 1O1" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 1771B8B4CE; Thu, 20 Jun 2019 17:22:04 +0000 (UTC) (envelope-from asomers@gmail.com) Received: by mail-lj1-f176.google.com with SMTP id a21so3441072ljh.7; Thu, 20 Jun 2019 10:22:04 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=LJqkFEbFVd9Oxp9J3o7kLyHxF301ZQFc/x3bgcAq73c=; b=kHiukNDGjD6Ir0TQAZq6VwInrCoPEJ7idEE9Unic4lf7OOwDGgLrgj6eWEXcplMazQ 9TQdI/DlXw1fBW8ud+B/Hrriw/zpvWbOwk9px524thjGtQYJ6vCznNavLh9SLZbny2A+ gJk1XtswPEVw1rFaxhR2oLpCQ3nuW07olHoLBmGFadYZQq9RFbcPYEaGH1bgK+AgdN5X 6uMdftWz8xh59qHpkxaERWk0UX3QENGP6izDCkZxXjET6e8QFG1FZ2rJ6nrtLkBM8ffU WTYGc8porevlM6Ca9MRb9MgCXN5XMFv5/0su9J1EWHV3/fBxSpSLrkNwqW+tm5TCHF8J 2OBw== X-Gm-Message-State: APjAAAX4lJngRB55x4nx5Rtc5ywMROOZ6RIg9nngg/bS9466A0uUaWP+ 6GKO5g0V7IiLsHlBwjSth4E48gvb6j6gYZ/SYk0= X-Google-Smtp-Source: APXvYqwdhf9e8j0fd4E2S+y4kqesRceM20E/EUbJwRji3s9tTeC7vXfrVDonooW2BFe4lIaW6SuhIXVwQhx29TMZ0Yw= X-Received: by 2002:a2e:1290:: with SMTP id 16mr50505738ljs.88.1561050925786; Thu, 20 Jun 2019 10:15:25 -0700 (PDT) MIME-Version: 1.0 References: <201906201435.x5KEZTqH021513@repo.freebsd.org> <54f3bc97cbb485cdcc44b81c82c149ac9e46d42f.camel@freebsd.org> <20190621013236.N5105@besplex.bde.org> In-Reply-To: <20190621013236.N5105@besplex.bde.org> From: Alan Somers Date: Thu, 20 Jun 2019 11:15:14 -0600 Message-ID: Subject: Re: svn commit: r349233 - head/sys/sys To: Bruce Evans Cc: Ian Lepore , src-committers , svn-src-all , svn-src-head Content-Type: text/plain; charset="UTF-8" X-Rspamd-Queue-Id: 1771B8B4CE X-Spamd-Bar: ------ Authentication-Results: mx1.freebsd.org X-Spamd-Result: default: False [-6.96 / 15.00]; NEURAL_HAM_MEDIUM(-1.00)[-0.996,0]; NEURAL_HAM_SHORT(-0.96)[-0.965,0]; REPLY(-4.00)[]; NEURAL_HAM_LONG(-1.00)[-1.000,0] X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 20 Jun 2019 17:22:06 -0000 On Thu, Jun 20, 2019 at 10:43 AM Bruce Evans wrote: > > On Thu, 20 Jun 2019, Alan Somers wrote: > > > On Thu, Jun 20, 2019 at 9:14 AM Ian Lepore wrote: > >> > >> On Thu, 2019-06-20 at 14:35 +0000, Alan Somers wrote: > >>> Author: asomers > >>> Date: Thu Jun 20 14:35:28 2019 > >>> New Revision: 349233 > >>> URL: https://svnweb.freebsd.org/changeset/base/349233 > >>> > >>> Log: > >>> #include from sys/filio.h > >>> > >>> This fixes world build after r349231 > > This increases the bugs in r349231 by adding more undocumented namespace > pollution. > > >>> Modified: head/sys/sys/filio.h > >>> ===================================================================== > >>> ========= > >>> --- head/sys/sys/filio.h Thu Jun 20 14:34:45 2019 (r349232) > >>> +++ head/sys/sys/filio.h Thu Jun 20 14:35:28 2019 (r349233) > >>> @@ -40,6 +40,7 @@ > >>> #ifndef _SYS_FILIO_H_ > >>> #define _SYS_FILIO_H_ > >>> > >>> +#include > >>> #include > >>> > >>> /* Generic file-descriptor ioctl's. */ > >> > >> I wonder... is this one of those situations where it is better to use > >> __int64_t in the struct, then #include ? I think the net > >> effect there would be less pollution with other types? I've never seen > >> written guidance about when to use the __names and _types.h, but I've > >> always had the general impression that if you have to include a header > >> from another system header, it's better to use the _header.h if it > >> exists. > > > > Good question. grep shows almost equal numbers of each (37 types.h > > and 33 _types.h) in sys/sys. Do you think I should change it? > > Only low quality headers include . > > r349231 adds a struct declaration with an int64_t in it. Everything is > undocumented of course, so one knows if this header declares all the > types needed to use it. If it only declared __int64_t and used that, > then users would have to know to use __int64_t or to include a header > that declares int64_t in order to use the struct properly (e.g., > bounds checking of the __int64_t member requires knowing its type). > This is a bit inconvenient. However, since everything is undocumented, > users have to read the header to see what is in it anyway, so they can > see that it uses __int64_t just as easily as they can see the name of > the struct member of that type. > > Higher quality headers usually declare all the standard types that they > use. Some even document the types that they declare. > > filio.h is not referenced in any man page. It is normally used by > including the omnibus header . is documented > well enough to require it to declare all the types that it uses. This > follows from the SYNOPSIS for ioctl(2) -- since no preqrequisites for > are given, it must not depend on other headers. > > ioctl(2) otherwise documents about 1% of the things needed to use it. > No one knows what macros and types defines/declares. > However, the headers included by used to be careful > about namespace pollution. They didn't do much more than #define lots > of macros and used only a few types and mostly only basic types. This > has rotted a bit. > > r349231 added the following bugs: > - use of int32_t, and no documentation of this > - namespace-polluting struct member names bn, runp and runb, and no > documentation of this > - comment not terminated by a ".". This is the first instance of this > bug in the file. > - space instead of tab after #define. This is the first instance of this > bug in the file. > > Previous bitrot in filio.h: > - namespace-polluting struct member names len and buf, and no documentation > of this. All struct member names in the file begin with fio, and this > would not be namespace pollution if it were documented. > - FIOSEEKDATA and FIOSEEKHOLE use the undeclared type off_t. This doesn't > break including , since there is only a problem if these > macros are used. These macros are of course undocumented, so the header > must be read to even know that they exist and then it is easy to see that > off_t must be declared to use them. > > Use of uint32_t and u_long, and function parameter names in the application > namespace are not problems since they are under a _KERNEL ifdef. > > The other headers included by are sys/ioccom.h, sys/sockio.h > and sys/ttycom.h: > > sys/ioccom.h: > Mostly implementation details; still fairly clean. > > sys/sockio.h: > Still defines only macros and doesn't define any of the types used by these > macros. Clearly, should _not_ declare all of these types, or > even one. Defining all of them might require including all socket and > network headers. Since there is no documentation, users must read this > header to see which type they need, then read the header that declares > that type to see what its prerequisites are... > > sys/ttycom.h: > Like sys/sockio.h, but a hundred times simpler. The only undeclared types > that it uses are struct timeval and struct termios. > > Summary: and the headers that it includes should declare > minimal types to compile (so __int64_t is enough). Most uses of this > header require including domain-specific headers which declare the > relevant data structures. > > Bruce Bruce, would you be satisfied by switching from to and from int64_t to __int64_t? -Alan