From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pj1-x1036.google.com (mail-pj1-x1036.google.com [IPv6:2607:f8b0:4864:20::1036]) by sourceware.org (Postfix) with ESMTPS id C61F33858C3A for ; Sat, 20 Nov 2021 00:11:45 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org C61F33858C3A Received: by mail-pj1-x1036.google.com with SMTP id gb13-20020a17090b060d00b001a674e2c4a8so10080313pjb.4 for ; Fri, 19 Nov 2021 16:11:45 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=hf2S1WJ4aFTn7w+Qf84ILTNTb79Hn7HiSsK8x/FAkNc=; b=kTc1quBpOeV5jT3CiUuLfW2rZOx7C0/OKOv9ix2h5fZbkl+RvGSTQj9x3qzWB5/xJE PBZsTNX0hkuVLLKWn+VDeXVQ/AR/wOVQKdFccVsOX61LpLLEDmnAyPH9yHiI2HzYf2VX B5jhCF6OqNnnN1pW1X5h1bDJE116QvLaviIt4mUFbQnThz76o78XP9MM76RdxrqCVCvq 60dK0k8c/rou/7EjeL+woyjAwq53SfZwB+zhncIbwvQRoo9d/v6pt0c8YTDijirL5MMk pTu/km2YnZj4ft3B9mdUNCP2PHgkgP/E3/OhRBVRizDMEirBKN3CPvcCNJaCIO8as6Mz UFbg== X-Gm-Message-State: AOAM533faMUFyVINFHM/bLzSktCtIuCBAlQxCiMkw9sSfAqGPXFeBcN6 3oQGCID0KhcNZM7PSWozEvbdrtSFWnc= X-Google-Smtp-Source: ABdhPJxfRDL0tryeJUt85wcgz94zi7MiGd2LRY5bVZlSPTdnWJMxtQnRi//ioYYbTOpWTHh5xd+rSA== X-Received: by 2002:a17:90a:c257:: with SMTP id d23mr4875077pjx.42.1637367104838; Fri, 19 Nov 2021 16:11:44 -0800 (PST) Received: from localhost ([2409:10:24a0:4700:e8ad:216a:2a9d:6d0c]) by smtp.gmail.com with ESMTPSA id t135sm566766pgc.51.2021.11.19.16.11.43 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 19 Nov 2021 16:11:44 -0800 (PST) Date: Sat, 20 Nov 2021 09:11:42 +0900 From: Stafford Horne To: Adhemerval Zanella Cc: libc-alpha@sourceware.org Subject: Re: [PATCH] linux: Add generic ioctl implementation Message-ID: References: <20211119195852.166123-1-adhemerval.zanella@linaro.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20211119195852.166123-1-adhemerval.zanella@linaro.org> X-Spam-Status: No, score=-10.5 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, GIT_PATCH_0, KAM_SHORT, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libc-alpha mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 20 Nov 2021 00:11:48 -0000 On Fri, Nov 19, 2021 at 04:58:52PM -0300, Adhemerval Zanella wrote: > The powerpc is refactor to use the default implementation. > --- > sysdeps/unix/sysv/linux/internal-ioctl.h | 23 +++++++ > sysdeps/unix/sysv/linux/ioctl.c | 49 +++++++++++++ > .../unix/sysv/linux/powerpc/internal-ioctl.h | 46 +++++++++++++ > sysdeps/unix/sysv/linux/powerpc/ioctl.c | 68 ------------------- > 4 files changed, 118 insertions(+), 68 deletions(-) > create mode 100644 sysdeps/unix/sysv/linux/internal-ioctl.h > create mode 100644 sysdeps/unix/sysv/linux/ioctl.c > create mode 100644 sysdeps/unix/sysv/linux/powerpc/internal-ioctl.h > delete mode 100644 sysdeps/unix/sysv/linux/powerpc/ioctl.c > > diff --git a/sysdeps/unix/sysv/linux/internal-ioctl.h b/sysdeps/unix/sysv/linux/internal-ioctl.h > new file mode 100644 > index 0000000000..697b086703 > --- /dev/null > +++ b/sysdeps/unix/sysv/linux/internal-ioctl.h > @@ -0,0 +1,23 @@ > +/* Linux internal definitions for ioctl. > + Copyright (C) 2021 Free Software Foundation, Inc. > + This file is part of the GNU C Library. > + > + The GNU C Library is free software; you can redistribute it and/or > + modify it under the terms of the GNU Lesser General Public > + License as published by the Free Software Foundation; either > + version 2.1 of the License, or (at your option) any later version. > + > + The GNU C Library is distributed in the hope that it will be useful, > + but WITHOUT ANY WARRANTY; without even the implied warranty of > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + Lesser General Public License for more details. > + > + You should have received a copy of the GNU Lesser General Public > + License along with the GNU C Library. If not, see > + . */ > + Should we have a comment here explaining why this is needed. Something like: /* Architecture ports may choose to override this default implementation to provide architecture specific ioctl support. For example see powerpc. */ > +static inline bool > +__ioctl_arch (int *r, int fd, unsigned long request, void *arg) > +{ > + return false; > +} > diff --git a/sysdeps/unix/sysv/linux/ioctl.c b/sysdeps/unix/sysv/linux/ioctl.c > new file mode 100644 > index 0000000000..c4855d6302 > --- /dev/null > +++ b/sysdeps/unix/sysv/linux/ioctl.c > @@ -0,0 +1,49 @@ > +/* Control device. Linux generic implementation. > + Copyright (C) 2021 Free Software Foundation, Inc. > + This file is part of the GNU C Library. > + > + The GNU C Library is free software; you can redistribute it and/or > + modify it under the terms of the GNU Lesser General Public > + License as published by the Free Software Foundation; either > + version 2.1 of the License, or (at your option) any later version. > + > + The GNU C Library is distributed in the hope that it will be useful, > + but WITHOUT ANY WARRANTY; without even the implied warranty of > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + Lesser General Public License for more details. > + > + You should have received a copy of the GNU Lesser General Public > + License along with the GNU C Library. If not, see > + . */ > + > +#include > +#include > +#include > +#include > + > +int > +__ioctl (int fd, unsigned long int request, ...) > +{ > + va_list args; > + va_start (args, request); > + void *arg = va_arg (args, void *); > + va_end (args); > + > + int r; > + if (!__ioctl_arch (&r, fd, request, arg)) > + { > + r = INTERNAL_SYSCALL_CALL (ioctl, fd, request, arg); > + if (__glibc_unlikely (INTERNAL_SYSCALL_ERROR_P (r))) > + { > + __set_errno (-r); > + return -1; > + } > + } > + return r; > +} > +libc_hidden_def (__ioctl) > +weak_alias (__ioctl, ioctl) > + > +#if __TIMESIZE != 64 > +strong_alias (__ioctl, __ioctl_time64) > +#endif For what its worth this looks good to me. > diff --git a/sysdeps/unix/sysv/linux/powerpc/internal-ioctl.h b/sysdeps/unix/sysv/linux/powerpc/internal-ioctl.h > new file mode 100644 > index 0000000000..3a2adce7c3 > --- /dev/null > +++ b/sysdeps/unix/sysv/linux/powerpc/internal-ioctl.h > @@ -0,0 +1,46 @@ > +/* Linux internal definitions for ioctl. > + Copyright (C) 2021 Free Software Foundation, Inc. > + This file is part of the GNU C Library. > + > + The GNU C Library is free software; you can redistribute it and/or > + modify it under the terms of the GNU Lesser General Public > + License as published by the Free Software Foundation; either > + version 2.1 of the License, or (at your option) any later version. > + > + The GNU C Library is distributed in the hope that it will be useful, > + but WITHOUT ANY WARRANTY; without even the implied warranty of > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + Lesser General Public License for more details. > + > + You should have received a copy of the GNU Lesser General Public > + License along with the GNU C Library. If not, see > + . */ > + > +#include > + > +static inline bool > +__ioctl_arch (int *r, int fd, unsigned long request, void *arg) > +{ > + switch (request) > + { > + case TCGETS: > + *r = __tcgetattr (fd, (struct termios *) arg); > + break; > + > + case TCSETS: > + *r = __tcsetattr (fd, TCSANOW, (struct termios *) arg); > + break; > + > + case TCSETSW: > + *r = __tcsetattr (fd, TCSADRAIN, (struct termios *) arg); > + break; > + > + case TCSETSF: > + *r = __tcsetattr (fd, TCSAFLUSH, (struct termios *) arg); > + break; > + > + default: > + return false; > + } > + return true; > +} > diff --git a/sysdeps/unix/sysv/linux/powerpc/ioctl.c b/sysdeps/unix/sysv/linux/powerpc/ioctl.c > deleted file mode 100644 > index a81c7ba54c..0000000000 > --- a/sysdeps/unix/sysv/linux/powerpc/ioctl.c > +++ /dev/null > @@ -1,68 +0,0 @@ > -/* Copyright (C) 1998-2021 Free Software Foundation, Inc. > - This file is part of the GNU C Library. > - > - The GNU C Library is free software; you can redistribute it and/or > - modify it under the terms of the GNU Lesser General Public > - License as published by the Free Software Foundation; either > - version 2.1 of the License, or (at your option) any later version. > - > - The GNU C Library is distributed in the hope that it will be useful, > - but WITHOUT ANY WARRANTY; without even the implied warranty of > - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > - Lesser General Public License for more details. > - > - You should have received a copy of the GNU Lesser General Public > - License along with the GNU C Library; if not, see > - . */ > - > -#include > -#include > -#include > -#include > -#include > - > -/* The user-visible size of struct termios has changed. Catch ioctl calls > - using the new-style struct termios, and translate them to old-style. */ > - > -int > -__ioctl (int fd, unsigned long int request, ...) > -{ > - void *arg; > - va_list ap; > - int result; > - > - va_start (ap, request); > - arg = va_arg (ap, void *); > - > - switch (request) > - { > - case TCGETS: > - result = __tcgetattr (fd, (struct termios *) arg); > - break; > - > - case TCSETS: > - result = __tcsetattr (fd, TCSANOW, (struct termios *) arg); > - break; > - > - case TCSETSW: > - result = __tcsetattr (fd, TCSADRAIN, (struct termios *) arg); > - break; > - > - case TCSETSF: > - result = __tcsetattr (fd, TCSAFLUSH, (struct termios *) arg); > - break; > - > - default: > - result = INLINE_SYSCALL (ioctl, 3, fd, request, arg); > - break; > - } > - > - va_end (ap); > - > - return result; > -} > -libc_hidden_def (__ioctl) > -weak_alias (__ioctl, ioctl) > -#if __TIMESIZE != 64 > -weak_alias (__ioctl, __ioctl_time64) > -#endif The powerpc bit look fine too. -Stafford