From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 69998 invoked by alias); 2 May 2018 10:46:00 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 69866 invoked by uid 89); 2 May 2018 10:45:43 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=BAYES_00,SPF_HELO_PASS,TIME_LIMIT_EXCEEDED autolearn=unavailable version=3.3.2 spammy=julio, Hx-languages-length:2436, renames X-HELO: mx1.redhat.com Received: from mx3-rdu2.redhat.com (HELO mx1.redhat.com) (66.187.233.73) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 02 May 2018 10:45:27 +0000 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.rdu2.redhat.com [10.11.54.6]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id D61B0EB715; Wed, 2 May 2018 10:45:22 +0000 (UTC) Received: from [127.0.0.1] (ovpn04.gateway.prod.ext.ams2.redhat.com [10.39.146.4]) by smtp.corp.redhat.com (Postfix) with ESMTP id 520F7215CDA7; Wed, 2 May 2018 10:45:17 +0000 (UTC) Subject: Re: [PATCH 3/4] Explicit fixed-width integral File I/O protocol types To: Julio Guerra , gdb-patches@sourceware.org References: <20180428011940.115515-1-julio@farjump.io> <20180428011940.115515-4-julio@farjump.io> Cc: Mike Frysinger From: Pedro Alves Message-ID: Date: Wed, 02 May 2018 10:46:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.6.0 MIME-Version: 1.0 In-Reply-To: <20180428011940.115515-4-julio@farjump.io> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-SW-Source: 2018-05/txt/msg00011.txt.bz2 On 04/28/2018 02:19 AM, Julio Guerra wrote: > The File I/O extension defines portable types of there host-specific > counterparts, such as `struct stat` and `struct timeval`. This patch improves > these type definitions in `include/gdb/fileio.h` to make possible sharing them > with target programs, and avoid redefining them by being able to include this > header, even with cross-compiled programs. > > The patch thus removes several drawbacks: > - avoid implicit pointers when defining fixed-width integers as array typedefs. > - explicitly state the sizes of fixed-width integers (e.g. fio_ulong_t becomes > fio_uint64_t). > It also renames a few misnamed conversion functions with the convention > `host_to_fileio_*` used everywhere else. > > Note that fixed-width integer types are defined using GCC's preprocessor builtin > macros to avoid using the libc's stdint.h which might not be available on the > target compiler. Therefore, `include/gdb/fileio.h` is standalone. Sorry, but NAK. I don't see how using GCC preprocessor builtins would be more portable than stdint.h. That doesn't make the file standalone -- it makes it dependent on gcc instead. But regardless, the major problem here is that this approach does not work, because it ignores the issue of alignment and padding holes. With the current approach, all structure fields are aligned to 1 byte. With the explicit types approach, fields are aligned to their natural alignment. E.g., from a glance, it seems like we'd be creating a 4-byte padding hole after fst_rdev. Even if by chance the fields end up aligned, that's not something we should be relying on. Both host and client must agree on the layout of the data structures. fio_stat objects are copied directly into target memory, see tail end of remote-fileio.c:remote_fileio_func_stat, for example: if (statptr) { host_to_fileio_stat (&st, &fst); host_to_fileio_uint (0, fst.fst_dev); errno = target_write_memory (statptr, (gdb_byte *) &fst, sizeof fst); if (errno != 0) As for the "int"/"long"/etc naming vs explicitly-sized uint32_t etc., note that the File I/O protocol's "int" "long" etc. types are defined here: https://sourceware.org/gdb/current/onlinedocs/gdb/Integral-Datatypes.html#Integral-Datatypes I don't have a strong opinion, but I'm not sure it's worth it to deviate from the documentation. Thanks, Pedro Alves