From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca (simark.ca [158.69.221.121]) by sourceware.org (Postfix) with ESMTPS id 9C23C3858D35 for ; Sat, 1 Aug 2020 14:45:04 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 9C23C3858D35 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=simark.ca Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=simark@simark.ca Received: from [10.0.0.11] (173-246-6-90.qc.cable.ebox.net [173.246.6.90]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id DA78D1E792; Sat, 1 Aug 2020 10:45:03 -0400 (EDT) Subject: Re: [PATCH] debuginfod-support.c: Replace globals with user_data To: Aaron Merey , gdb-patches@sourceware.org References: From: Simon Marchi Message-ID: Date: Sat, 1 Aug 2020 10:44:56 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.10.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: fr Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-9.2 required=5.0 tests=BAYES_00, GIT_PATCH_0, KAM_DMARC_STATUS, NICE_REPLY_A, SPF_HELO_PASS, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 01 Aug 2020 14:45:05 -0000 > From 1913317cb95d02c3244e5d91b7b59e33cfe991c5 Mon Sep 17 00:00:00 2001 > From: Aaron Merey > Date: Fri, 31 Jul 2020 18:25:48 -0400 > Subject: [PATCH] debuginfod-support.c: Replace globals with user_data > > Store query information in user_data struct instead of global > variables. Also include DEBUGINFOD_CFLAGS in INTERNAL_CFLAG_BASE. The DEBUGINFOD_CFLAGS change looks good, but it's unrelated to the topic of this patch, please make it its own patch, with its own justification. > diff --git a/gdb/debuginfod-support.c b/gdb/debuginfod-support.c > index f4a227b040..9325bf8651 100644 > --- a/gdb/debuginfod-support.c > +++ b/gdb/debuginfod-support.c > @@ -43,29 +43,39 @@ debuginfod_debuginfo_query (const unsigned char *build_id, > #else > #include > > -/* TODO: Use debuginfod API extensions instead of these globals. */ > -static std::string desc; > -static std::string fname; > -static bool has_printed; > +struct user_data > +{ > + std::string desc; > + std::string fname; > + bool has_printed; > + > + user_data (std::string desc, std::string fname, bool has_printed) > + : desc (desc), fname (fname), has_printed (has_printed) > + { } Put the constructor before the fields. Make the fields const if they are not intended to be changed once the object has been created. Using `std::string` causes unnecessary copying to happen. Just use `const char *` (`const char * const` for the fields) or gdb::string_view, if you want to be more generic (but all callers are `const char *`, so it's not really necessary). That's assuming that the lifetime of those strings will always be longer than the lifetime of the debuginfo client. I think that's the case, since the debug info client is always created and destroyed in the same function. > +}; > > static int > progressfn (debuginfod_client *c, long cur, long total) > { > + struct user_data *data = static_cast (debuginfod_get_user_data (c)); This line is too long, just skip the `struct` keyword and you'll be fine. Also, space before `*`. > + > if (check_quit_flag ()) > { > printf_filtered ("Cancelling download of %s %ps...\n", > - desc.c_str (), > - styled_string (file_name_style.style (), fname.c_str ())); > + data->desc.c_str (), > + styled_string (file_name_style.style (), > + data->fname.c_str ())); > return 1; > } > > - if (!has_printed && total != 0) > + if (! data->has_printed && total != 0) Remove space after `!`. Simon