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 43545385E02B for ; Mon, 13 Apr 2020 20:11:51 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 43545385E02B 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] (unknown [192.222.164.54]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id C14A41E60C; Mon, 13 Apr 2020 16:11:50 -0400 (EDT) Subject: Re: [PATCH] Implement IP_STAT on NetBSD To: Kamil Rytarowski , gdb-patches@sourceware.org References: <20200413181911.17133-1-n54@gmx.com> <64ef8889-0782-a541-6efa-3c37ed06b372@simark.ca> <4671cd7b-b081-1aad-88c3-f40aaefa38f5@gmx.com> From: Simon Marchi Message-ID: <52edb779-5a48-ccd0-4873-67551aa89eac@simark.ca> Date: Mon, 13 Apr 2020 16:11:49 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.6.0 MIME-Version: 1.0 In-Reply-To: <4671cd7b-b081-1aad-88c3-f40aaefa38f5@gmx.com> Content-Type: text/plain; charset=utf-8 Content-Language: fr Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-11.5 required=5.0 tests=BAYES_00, KAM_DMARC_STATUS, 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: Mon, 13 Apr 2020 20:11:52 -0000 On 2020-04-13 3:43 p.m., Kamil Rytarowski wrote: >>> @@ -433,6 +568,149 @@ nbsd_nat_target::info_proc (const char *args, enum info_proc_what what) >>> else >>> warning (_("unable to fetch virtual memory map")); >>> } >>> + if (do_stat) >>> + { >>> + /* First, try with Linux-compat /proc//stat. */ >>> + char filename[100]; >>> + xsnprintf (filename, sizeof filename, "/proc/%d/stat", pid); >> >> Use >> >> std::string filename = string_printf (...); >> > > OK. Meanwhile linux-tdep.c could be upgraded by a Linux maintainer. Ok >>> + gdb::unique_xmalloc_ptr statstr >>> + = target_fileio_read_stralloc (NULL, filename); >>> + >>> + /* Then fallback to emulating /proc//stat with sysctl(3). */ >>> + if (statstr == nullptr) >>> + statstr = nbsd_pid_to_statstr (pid); >> >> I find it a bit strange to take the route of formatting the contents of kinfo_proc2, >> only to parse it here. I think it would have been more natural to do: >> >> - If /proc/#/stat is available: string -> struct some_internal_struct -> print function >> - If /proc/#/stat is available: kinfo_proc2 -> struct some_internal_struct -> print function >> > > > This compat shim is only to emulate Linux... it's possible to follow the > FreeBSD case and just print local kinfo_proc2 as-is (that is fully > OS-specific). > > What do you think? > > The same question applies to IP_STATUS. When you put it like this... it just seems useless to read the info from /proc. If sysctl is guaranteed to always be there and is the blessed way of getting the information, then let's just use that. The goal is not to emulate Linux, but to use each OS's interface to get the required information. >>> +#if 0 /* Don't know how architecture-dependent the rest is... >>> + Anyway the signal bitmap info is available from "status". */ >> >> Either figure out how to print this correctly, or let's just leave it out. >> > > This comment was copied as-is from gdb/linux-tdep.c. In my opinion it's not great to leave this as-is in linux-tdep.c either, so I'd suggest to avoid spreading it. Simon