From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 43128 invoked by alias); 2 Dec 2019 22:16:14 -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 43108 invoked by uid 89); 2 Dec 2019 22:16:09 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-14.1 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_2,GIT_PATCH_3 autolearn=ham version=3.3.1 spammy=imagined 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; Mon, 02 Dec 2019 22:16:07 +0000 Received: by mx1.osci.io (Postfix, from userid 994) id A2A0A20362; Mon, 2 Dec 2019 17:16:05 -0500 (EST) Received: from gnutoolchain-gerrit.osci.io (gnutoolchain-gerrit.osci.io [8.43.85.239]) by mx1.osci.io (Postfix) with ESMTP id 072842032B; Mon, 2 Dec 2019 17:16:02 -0500 (EST) Received: from localhost (localhost [127.0.0.1]) by gnutoolchain-gerrit.osci.io (Postfix) with ESMTP id AE03E2816F; Mon, 2 Dec 2019 17:16:02 -0500 (EST) X-Gerrit-PatchSet: 3 Date: Mon, 02 Dec 2019 22:16:00 -0000 From: "Andrew Burgess (Code Review)" To: Luis Machado , gdb-patches@sourceware.org Cc: Simon Marchi Auto-Submitted: auto-generated X-Gerrit-MessageType: comment Subject: [review v3] [ARM, sim] Fix build error and warnings X-Gerrit-Change-Id: I21db699d3b61b2de8c44053e47be4387285af28f X-Gerrit-Change-Number: 726 X-Gerrit-ChangeURL: X-Gerrit-Commit: 3225e5889d2e0ced69a230eb423d976c8fc4a338 In-Reply-To: References: X-Gerrit-Comment-Date: Mon, 2 Dec 2019 17:16:02 -0500 Reply-To: gnutoolchain-gerrit@osci.io MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Content-Disposition: inline User-Agent: Gerrit/3.0.3-79-g83ff7f88f1 Content-Type: text/plain; charset=UTF-8 Message-Id: <20191202221602.AE03E2816F@gnutoolchain-gerrit.osci.io> X-SW-Source: 2019-12/txt/msg00063.txt.bz2 Andrew Burgess has posted comments on this change. Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/726 ...................................................................... Patch Set 3: (3 comments) | --- sim/arm/wrapper.c | +++ sim/arm/wrapper.c | @@ -126,16 +126,16 @@ }; | | union maverick_acc_regs | { | long double ld; /* Acc registers are 72-bits. */ | }; | | -struct maverick_regs DSPregs[16]; | -union maverick_acc_regs DSPacc[4]; | -ARMword DSPsc; | +extern struct maverick_regs DSPregs[16]; PS1, Line 132: No, I imagined moving the declaration into the header file, so they are declared once and defined once. Yes I agree that this code is a mess. If you can figure out a better header file to place this into that's fine, but just creating a new one seemed like the least effort, but leaves things so we don't have duplicate type definitions and variable declarations. | +extern union maverick_acc_regs DSPacc[4]; | +extern ARMword DSPsc; | | static void | init (void) | { | static int done; | | if (!done) ... | @@ -231,18 +231,18 @@ sim_resume (SIM_DESC sd ATTRIBUTE_UNUSED, | SIM_RC | sim_create_inferior (SIM_DESC sd ATTRIBUTE_UNUSED, | struct bfd * abfd, | char * const *argv, | char * const *env) | { | int argvlen = 0; | int mach; | - char **arg; | + char * const *arg; PS1, Line 239: You're absolutely right, I missread the diff, my problem isn't with this at all, it's with the next hunk... | | init (); | | if (abfd != NULL) | { | ARMul_SetPC (state, bfd_get_start_address (abfd)); | mach = bfd_get_mach (abfd); | } | else ... | @@ -862,18 +862,18 @@ sim_open (SIM_OPEN_KIND kind, | CPU_REG_FETCH (cpu) = arm_reg_fetch; | CPU_REG_STORE (cpu) = arm_reg_store; | CPU_PC_FETCH (cpu) = arm_pc_get; | CPU_PC_STORE (cpu) = arm_pc_set; | } | | sim_callback = cb; | | - sim_target_parse_arg_array (argv); | + sim_target_parse_arg_array ((char **) argv); PS1, Line 870: So this one I don't like. I wondered if we couldn't simply push the const-ness into the functions being called, but then we end up in sim_target_parse_command_line where the ARGV array is modified. What I really don't understand right now is why the ARGV array needs to be modified in sim_target_parse_command_line at all - once we return from sim_open I don't think that the ARGV array is used any further. And reading the comments in gdb/remote-sim.c relating to the use of ARGVs, I suspect that modifying this would be a bad thing. I think we need to understand more about how the argv array is being used in order to solve this issue properly. | | if (argv[1] != NULL) | { | int i; | | /* Scan for memory-size switches. */ | for (i = 0; (argv[i] != NULL) && (argv[i][0] != 0); i++) | if (argv[i][0] == '-' && argv[i][1] == 'm') | { -- Gerrit-Project: binutils-gdb Gerrit-Branch: master Gerrit-Change-Id: I21db699d3b61b2de8c44053e47be4387285af28f Gerrit-Change-Number: 726 Gerrit-PatchSet: 3 Gerrit-Owner: Luis Machado Gerrit-Reviewer: Andrew Burgess Gerrit-Reviewer: Luis Machado Gerrit-CC: Simon Marchi Gerrit-Comment-Date: Mon, 02 Dec 2019 22:16:02 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Luis Machado Comment-In-Reply-To: Andrew Burgess Comment-In-Reply-To: Simon Marchi Gerrit-MessageType: comment