From patchwork Fri Jun 29 14:01:29 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Julio Guerra X-Patchwork-Id: 28152 Received: (qmail 57639 invoked by alias); 29 Jun 2018 14:01:41 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 57262 invoked by uid 89); 29 Jun 2018 14:01:40 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-24.1 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, HTML_MESSAGE, KAM_SHORT, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.2 spammy=H*c:alternative X-HELO: a2-134.smtp-out.eu-west-1.amazonses.com Received: from a2-134.smtp-out.eu-west-1.amazonses.com (HELO a2-134.smtp-out.eu-west-1.amazonses.com) (54.240.2.134) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 29 Jun 2018 14:01:32 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/simple; s=jl7vyxitgsfircfdhxflkj2c3tgxidze; d=farjump.io; t=1530280889; h=Subject:From:To:Date:Mime-Version:Content-Type:In-Reply-To:References:Message-Id; bh=LPb4ibwUFFa2GknwFBgbvzCWuELfb6CPYTLRRjpUmlE=; b=B//hzkFGPIMSciirUkRRRA5RlgcjN7Gn/p3rLCn51RYZjJZ78rlbLpGYyPgWmDnT BhAzw96wfcfCZ+ajNeMpWDSU+QF87shBl+KYGPr/t+fTgxGEUx1W3YxdG5i72C5IGgx xqBoNo61pwOmhu68yOS9shieQ+gdZdQ1PFKm//Wg= DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/simple; s=uku4taia5b5tsbglxyj6zym32efj7xqv; d=amazonses.com; t=1530280889; h=Subject:From:To:Date:Mime-Version:Content-Type:In-Reply-To:References:Message-Id:Feedback-ID; bh=LPb4ibwUFFa2GknwFBgbvzCWuELfb6CPYTLRRjpUmlE=; b=PAdrofv1XrHSwh37UzSedPT9wAcZoPYmGJHFAQ8Xs62zh6JsaXJ1bQwj8jqAsWNY t8jvey6wSOXLGh0fGmcHLg27XiNhmtlYK6JGgbrIgWXv/NQbaMUKnRtqMGf63pb52ZB HlNDaCkyotSxMocN+yqB7XLaXxIFvKPiPxFhdAp8= Subject: Re: [PATCH v3] Allow using special files with File I/O functions From: =?UTF-8?Q?Julio_Guerra?= To: =?UTF-8?Q?Pedro_Alves?= , =?UTF-8?Q?gdb-patches=40s?= =?UTF-8?Q?ourceware=2Eorg?= Date: Fri, 29 Jun 2018 14:01:29 +0000 Mime-Version: 1.0 In-Reply-To: <3c1caf7f-f50f-a8d7-43f3-f8fa8eca663d@redhat.com> References: <20180628192635.44056-1-julio@farjump.io> <0102016447dcf9e9-3989bcd9-1272-4a05-93c5-77823c7a0921-000000@eu-west-1.amazonses.com> <3c1caf7f-f50f-a8d7-43f3-f8fa8eca663d@redhat.com> <79758ca1-2541-9ae6-d793-b367d6094468@farjump.io> Message-ID: <010201644bd94c4b-0d8759a9-5625-4773-a858-6e218d4fc9db-000000@eu-west-1.amazonses.com> Le 29/06/2018 à 15:42, Pedro Alves a écrit : On 06/28/2018 08:27 PM, Julio Guerra wrote: - Remove the restriction to regular files only and add support for special file types in the File IO stat structure. - Define a few more macro definitions of file types such as FIFOs, etc. The major goal is being able to write advanced embedded testing functions, like: - using a FIFO between the embedded program and the host, instead of being restricted only to the GDB console. - mocking features based on host's by opening some /dev special files. This needs a GDB manual change, and a NEWS entry. Ok. 2018-06-28 Julio Guerra * remote-fileio.c (remote_fileio_func_open, remote_fileio_func_stat): Allow using File I/O functions open(), stat() and fstat() on special files. * ../include/gdb/fileio.h: Add macro definitions for special files, both for fst_dev and fst_mode fields of struct fst_stat. * common/fileio.c (fileio_to_host_mode, fileio_mode_pack): Add new special file types in fst_mode's definition. (host_to_fileio_stat): Define fst_dev using the new macro definitions and according to the file's type. Note that include/ has its own ChangeLog file. Ok. 2018-06-27 Simon Marchi * gdb-gdb.py.in (StructMainTypePrettyPrinter) : Don't EISDIR: The named file is a directory and oflag includes O_WRONLY or O_RDWR, or includes O_CREAT without O_DIRECTORY. I wait for your comments until I resubmit a v4. Thank you, diff --git a/gdb/common/fileio.c b/gdb/common/fileio.c index 912a7ede3c..9ee78e227c 100644 --- a/gdb/common/fileio.c +++ b/gdb/common/fileio.c @@ -119,12 +119,31 @@ fileio_to_host_mode (int fileio_mode, mode_t *mode_p) if (fileio_mode & ~FILEIO_S_SUPPORTED) return -1; - if (fileio_mode & FILEIO_S_IFREG) - mode |= S_IFREG; - if (fileio_mode & FILEIO_S_IFDIR) - mode |= S_IFDIR; - if (fileio_mode & FILEIO_S_IFCHR) - mode |= S_IFCHR; + switch (fileio_mode & FILEIO_S_IFMT) + { + case FILEIO_S_IFSOCK: + *mode_p |= S_IFSOCK; + break; + case FILEIO_S_IFLNK: + mode |= S_IFLNK; + break; + case FILEIO_S_IFREG: + mode |= S_IFREG; + break; + case FILEIO_S_IFBLK: + mode |= S_IFBLK; + break; + case FILEIO_S_IFDIR: + mode |= S_IFDIR; + break; + case FILEIO_S_IFCHR: + mode |= S_IFCHR; + break; + case FILEIO_S_IFIFO: + mode |= S_IFIFO; + break; + } + if (fileio_mode & FILEIO_S_IRUSR) mode |= S_IRUSR; if (fileio_mode & FILEIO_S_IWUSR) @@ -165,12 +184,31 @@ fileio_mode_pack (mode_t mode) { mode_t tmode = 0; - if (S_ISREG (mode)) - tmode |= FILEIO_S_IFREG; - if (S_ISDIR (mode)) - tmode |= FILEIO_S_IFDIR; - if (S_ISCHR (mode)) - tmode |= FILEIO_S_IFCHR; + switch (mode & S_IFMT) + { + case S_IFSOCK: + tmode |= FILEIO_S_IFSOCK; + break; + case S_IFLNK: + tmode |= FILEIO_S_IFLNK; + break; + case S_IFREG: + tmode |= FILEIO_S_IFREG; + break; + case S_IFBLK: + tmode |= FILEIO_S_IFBLK; + break; + case S_IFDIR: + tmode |= FILEIO_S_IFDIR; + break; + case S_IFCHR: + tmode |= FILEIO_S_IFCHR; + break; + case S_IFIFO: + tmode |= FILEIO_S_IFIFO; + break; + } I'm not sure whether all these S_FOO macros exist on all hosts. Looking at: https://www.gnu.org/software/gnulib/manual/html_node/sys_002fstat_002eh.html gnulib's sys/stat.h (gdb/gnulib/import/sys_stat.in.h) adds some replacements, but then I'm not sure whether checking S_IFxxx etc. directly instead of using the S_ISxxx function-style macros is the right thing to do portability-wise. It may be S_ISxxx was being used for a reason? I assumed a system >= POSIX.1-2001 here. man 7 inode says: The S_IF* constants are present in POSIX.1-2001 and later. […] The S_ISLNK() and S_ISSOCK() macros were not in POSIX.1-1996, but both are present in POSIX.1-2001 + if (mode & S_IRUSR) tmode |= FILEIO_S_IRUSR; if (mode & S_IWUSR) @@ -224,8 +262,10 @@ void host_to_fileio_stat (struct stat *st, struct fio_stat *fst) { LONGEST blksize; + long fst_dev; We don't align variable names like that. Ok. - host_to_fileio_uint ((long) st->st_dev, fst->fst_dev); + fst_dev = S_ISREG(st->st_mode) ? FILEIO_STDEV_FILE : FILEIO_STDEV_SPECIAL; Missing space in "S_ISREG (". Ok. + host_to_fileio_uint (fst_dev, fst->fst_dev); host_to_fileio_uint ((long) st->st_ino, fst->fst_ino); host_to_fileio_mode (st->st_mode, fst->fst_mode); host_to_fileio_uint ((long) st->st_nlink, fst->fst_nlink); diff --git a/gdb/remote-fileio.c b/gdb/remote-fileio.c index 313da642ea..837081269a 100644 --- a/gdb/remote-fileio.c +++ b/gdb/remote-fileio.c @@ -407,24 +407,6 @@ remote_fileio_func_open (remote_target *remote, char *buf) return; } - /* Check if pathname exists and is not a regular file or directory. If so, - return an appropriate error code. Same for trying to open directories - for writing. */ Did you intend to remove the "Same for trying to open directories for writing." part? Yes, because of man 2 open: