From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 104765 invoked by alias); 7 Nov 2019 11:05:34 -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 104742 invoked by uid 89); 7 Nov 2019 11:05:33 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-8.7 required=5.0 tests=AWL,BAYES_00 autolearn=ham version=3.3.1 spammy= X-HELO: mx1.osci.io Received: from polly.osci.io (HELO mx1.osci.io) (8.43.85.229) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 07 Nov 2019 11:05:30 +0000 Received: by mx1.osci.io (Postfix, from userid 994) id 029B020220; Thu, 7 Nov 2019 06:05:28 -0500 (EST) Received: from gnutoolchain-gerrit.osci.io (gnutoolchain-gerrit.osci.io [8.43.85.239]) by mx1.osci.io (Postfix) with ESMTP id 3FD4720322; Thu, 7 Nov 2019 06:05:24 -0500 (EST) Received: from localhost (localhost [127.0.0.1]) by gnutoolchain-gerrit.osci.io (Postfix) with ESMTP id DAECE25B28; Thu, 7 Nov 2019 06:05:23 -0500 (EST) X-Gerrit-PatchSet: 2 Date: Thu, 07 Nov 2019 11:05:00 -0000 From: "Andrew Burgess (Code Review)" To: gdb-patches@sourceware.org Cc: Pedro Alves , Simon Marchi , Tom Tromey Auto-Submitted: auto-generated X-Gerrit-MessageType: comment Subject: [review v2] gdb: Support printf 'z' size modifier X-Gerrit-Change-Id: Ib6c44d88aa5bce265d757e4c0698881803dd186f X-Gerrit-Change-Number: 511 X-Gerrit-ChangeURL: X-Gerrit-Commit: 79c86dc68e08472600ffa71fb6d4cd7f5a0379dc In-Reply-To: References: X-Gerrit-Comment-Date: Thu, 7 Nov 2019 06:05:23 -0500 Reply-To: gnutoolchain-gerrit@osci.io MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Content-Disposition: inline User-Agent: Gerrit/3.0.3-75-g9005159e5d Content-Type: text/plain; charset=UTF-8 Message-Id: <20191107110523.DAECE25B28@gnutoolchain-gerrit.osci.io> X-SW-Source: 2019-11/txt/msg00209.txt.bz2 Andrew Burgess has posted comments on this change. Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/511 ...................................................................... Patch Set 2: (3 comments) New version should hopefully correctly handle the argument as a different size and includes some selftests. https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/511/1//COMMIT_MSG Commit Message: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/511/1//COMMIT_MSG@11 PS1, Line 11: 6 | 7 | gdb: Support printf 'z' size modifier 8 | 9 | The gdb format mechanism doesn't currently support the 'z' size 10 | modifier, there are a few places in GDB where this is used. Instead 11 > of removing these uses lets just add support to GDB for using 'z'. 12 | 13 | I've not added a test for this as I ran into the issue when trying to 14 | use some debug output. Before this commit: 15 | 16 | (gdb) set debug dwarf-line 9 > Just FYI, we didn't use to allow 'z' since it is a C99 feature (and thus C++11), and older compilers […] I did consider just replacing all uses of %z with something else and casting the argument, there aren't that many uses so the patch would be pretty similar in size to what I've now posted. Let me know if you'd prefer this approach. https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/511/1//COMMIT_MSG@13 PS1, Line 13: 8 | 9 | The gdb format mechanism doesn't currently support the 'z' size 10 | modifier, there are a few places in GDB where this is used. Instead 11 | of removing these uses lets just add support to GDB for using 'z'. 12 | 13 > I've not added a test for this as I ran into the issue when trying to 14 > use some debug output. Before this commit: 15 | 16 | (gdb) set debug dwarf-line 9 17 | (gdb) file test 18 | Reading symbols from test... 19 | Unrecognized format specifier 'z' in printf > It would seem easy enough to add an entry in test_format_specifier, in unittests/format_pieces-selft […] Done. https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/511/1/gdb/gdbsupport/format.c File gdb/gdbsupport/format.c: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/511/1/gdb/gdbsupport/format.c@237 PS1, Line 237: 23 | format_pieces::format_pieces (const char **arg, bool gdb_extensions) | ... 232 | seen_double_big_d = 1; 233 | } 234 | else 235 | seen_big_d = 1; 236 | } 237 > /* For size_t or ssize_t. */ 238 > else if (*f == 'z') 239 > f++; 240 | 241 | switch (*f) 242 | { 243 | case 'u': 244 | if (seen_hash) > This seems necessary but perhaps not sufficient -- nothing […] Thanks for this. I think I was fooled by it "just working" when treating the size_t as an int. I believe I've now handled this correctly. -- Gerrit-Project: binutils-gdb Gerrit-Branch: master Gerrit-Change-Id: Ib6c44d88aa5bce265d757e4c0698881803dd186f Gerrit-Change-Number: 511 Gerrit-PatchSet: 2 Gerrit-Owner: Andrew Burgess Gerrit-Reviewer: Andrew Burgess Gerrit-CC: Pedro Alves Gerrit-CC: Simon Marchi Gerrit-CC: Tom Tromey Gerrit-Comment-Date: Thu, 07 Nov 2019 11:05:23 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Pedro Alves Comment-In-Reply-To: Tom Tromey Comment-In-Reply-To: Simon Marchi Gerrit-MessageType: comment