From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-sender-0.a4lg.com (mail-sender-0.a4lg.com [IPv6:2401:2500:203:30b:4000:6bfe:4757:0]) by sourceware.org (Postfix) with ESMTPS id BF437382EA3D for ; Thu, 27 Oct 2022 01:29:15 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org BF437382EA3D Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=irq.a4lg.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=irq.a4lg.com Received: from [127.0.0.1] (localhost [127.0.0.1]) by mail-sender-0.a4lg.com (Postfix) with ESMTPSA id 4EC78300089; Thu, 27 Oct 2022 01:29:13 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=irq.a4lg.com; s=2017s01; t=1666834153; bh=VRGmEgRl0LoteyarjfxP+IpboNrbIUwe3seePNzN2/c=; h=Message-ID:Date:Mime-Version:Subject:To:References:From: In-Reply-To:Content-Type:Content-Transfer-Encoding; b=mQMTWhJZ2NPxMJHZ8daoo57BL0L9BJVfWvxGDRf7lguqVoTEUKc6A8TCHdojoQB7H tpTtvTMbqirkRw2A/YdMuDtDnIlWSnff2yAqY1XOIy0ThttpUgN0xWMMAm2C+D9poh 8R5wHRyiOlPawEHrKisgjpgqDfXkgVW67msglLAw= Message-ID: Date: Thu, 27 Oct 2022 10:29:11 +0900 Mime-Version: 1.0 Subject: Re: [PATCH v2] sim, sim/{m32c,ppc,rl78}: Use getopt_long Content-Language: en-US To: Mike Frysinger , gdb-patches@sourceware.org References: <24e83e920d728237c4efe6f4720643d6fbbf1084.1666113214.git.research_trasio@irq.a4lg.com> <7ad71357e72129e5dc642a5233868b3aa81c484c.1666679042.git.research_trasio@irq.a4lg.com> <8d3126e7-d6ed-fd03-4956-0226e099ab48@irq.a4lg.com> From: Tsukasa OI In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-12.1 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,GIT_PATCH_0,SPF_HELO_NONE,SPF_PASS,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On 2022/10/27 1:01, Mike Frysinger wrote: > On 26 Oct 2022 19:57, Tsukasa OI wrote: >> On 2022/10/26 17:59, Mike Frysinger wrote: >>> On 25 Oct 2022 06:27, Tsukasa OI wrote: >>>> Because of Binutils/GCC hack, getopt on GNU libc (2.25 or earlier) is >>>> currently unusable on sim, causing a regression on CentOS 7. >>>> >>>> This is caused as follows: >>>> >>>> 1. If HAVE_DECL_GETOPT is defined (getopt with known prototype is >>>> declared), a declaration of getopt in "include/getopt.h" is suppressed. >>>> The author started to define HAVE_DECL_GETOPT in sim with the commit >>>> 340aa4f6872c ("sim: Check known getopt definition existence"). >>>> 2. GNU libc (2.25 or earlier)'s includes to declare >>>> getopt function (only, not getopt_long or getopt_long_only) but it >>>> causes to include Binutils/GCC's "include/getopt.h". >>>> 3. If both 1. and 2. are satisfied, despite that tries to >>>> declare getopt by including , "include/getopt.h" does not >>>> define one, causing getopt function unusable. >>>> >>>> Getting rid of "include/getopt.h" (e.g. renaming this header file) is the >>>> best solution to avoid hacking but as a short-term solution, this commit >>>> replaces getopt with getopt_long under sim/. >>>> --- >>>> sim/igen/igen.c | 6 ++++-- >>>> sim/m32c/main.c | 5 ++++- >>>> sim/ppc/dgen.c | 6 ++++-- >>>> sim/ppc/igen.c | 9 ++++++--- >>>> sim/rl78/main.c | 4 +++- >>>> 5 files changed, 21 insertions(+), 9 deletions(-) >>>> >>>> diff --git a/sim/igen/igen.c b/sim/igen/igen.c >>>> index ba856401fa9..22cfd30ec43 100644 >>>> --- a/sim/igen/igen.c >>>> +++ b/sim/igen/igen.c >>>> @@ -989,6 +989,7 @@ main (int argc, char **argv, char **envp) >>>> char *real_file_name = NULL; >>>> int is_header = 0; >>>> int ch; >>>> + struct option dummy_longopts = { 0 }; >>> >>> just call it "longopts" so we don't have to rename it in the future if we >>> decide to actually add long options. comes up in the other files too. >>> >>> otherwise lgtm. >>> -mike >> >> To prepare actual long options, not just renaming, making them array of >> struct option seems better. Moving longopts out from the caller is >> avoided for now (since it might not get big so that it requires option >> definition outside a function). That means... >> >> Before: struct option dummy_longopts = { 0 }; >> After: struct option longopts[] = { { 0 } }; >> >> I fixed like this and I'll submit PATCH v3 (with fix above and minor >> commit message clarification) soon. Finally, I can clean up the mess I >> created. > > i would make it static const too in that case so it isn't constructed on > the stack ... it's just part of the rodata and loaded right away. That makes sense. I submitted PATCH v4: and I will push this version in this weekend unless there's an objection from someone. > >> Ah, a minor question to you, Mike. Can I consider your "lgtm" as an >> approval for specific area you are responsible? > > "lgtm" is equiv to "approved, please push when you're ready" > -mike Thanks! I will push some of my "build with Clang" work with plain "lgtm" response in this weekend. Tsukasa