From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id uK3XK2GMh2er1g8AWB0awg (envelope-from ) for ; Wed, 15 Jan 2025 05:22:25 -0500 Authentication-Results: simark.ca; dkim=pass (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=WOCsSqUL; dkim-atps=neutral Received: by simark.ca (Postfix, from userid 112) id B0E0D1E100; Wed, 15 Jan 2025 05:22:25 -0500 (EST) X-Spam-Checker-Version: SpamAssassin 4.0.0 (2022-12-13) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-6.4 required=5.0 tests=ARC_SIGNED,ARC_VALID,BAYES_00, DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED autolearn=unavailable autolearn_force=no version=4.0.0 Received: from server2.sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (prime256v1) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id 547641E08E for ; Wed, 15 Jan 2025 05:22:25 -0500 (EST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 4F34F385DC29 for ; Wed, 15 Jan 2025 10:22:24 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 4F34F385DC29 Authentication-Results: sourceware.org; dkim=pass (1024-bit key, unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=WOCsSqUL Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTP id ED7C3385DDCD for ; Wed, 15 Jan 2025 10:21:46 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org ED7C3385DDCD Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org ED7C3385DDCD Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.133.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1736936507; cv=none; b=H3e9FMEy+7jL3s3Yv+6ec/YTa6w51v9AJFDqGK4V0gt8iL8yrs4ickslLhgU94FZn4IX362q/AX9ieYMqcIJuRsWoidqvEyswdOew07WZlnSoye+viCNeNBX59ubbtzHb/QpNvUhkjUPzyZrMbcXaNDP1h9tU07YSeb45rzZSdI= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1736936507; c=relaxed/simple; bh=NQhnQ2E7jrHodQY8S5U+houC6mgtpJcNouSzMWamULg=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=DSHBia7AaDwTym4/E5NWJGD/299ox0O9IRH4/OHsLRXHGsf10ze5wmkC9u/VuWVAZ9OdXOua4OFv+eALSFfUpDtVoDOq0py1RN24B86uYPxoO92eUa8OdyS7T76+3Mh5DwDaav+EKDja5MRFW+owU6ugH00pMCFRUSeXEeyr0u4= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org ED7C3385DDCD DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1736936506; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=90mBSGl7pcebVJokiPOLWu8XfnBxkX3fOYgc7abLUgM=; b=WOCsSqULKwle5GPSEbF+kou0EoQ6vU8xUpV1wZgDeOVPhn6ZJcq3YZLjcdjTCWuRlZxJVD uRGvgPpYDEpC1HNcsS4aXJMha5x23JRA8PyKmG2v6R6x4pH5PsIj8vA0uYowHIBwZnxgDP LKoEV3iJwC68YY4MSZt/zWZumOsrfyQ= Received: from mail-wm1-f70.google.com (mail-wm1-f70.google.com [209.85.128.70]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-504-leQJM0QzNPWE2vCMDDaujQ-1; Wed, 15 Jan 2025 05:21:45 -0500 X-MC-Unique: leQJM0QzNPWE2vCMDDaujQ-1 X-Mimecast-MFC-AGG-ID: leQJM0QzNPWE2vCMDDaujQ Received: by mail-wm1-f70.google.com with SMTP id 5b1f17b1804b1-4362153dcd6so34015275e9.2 for ; Wed, 15 Jan 2025 02:21:45 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1736936504; x=1737541304; h=mime-version:message-id:date:references:in-reply-to:subject:cc:to :from:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=90mBSGl7pcebVJokiPOLWu8XfnBxkX3fOYgc7abLUgM=; b=iEdntjMorp+bsaCI5AmyoLuQYVfMk3YR6TXmDpiNUkRvBTMTy43PRXV4m53pCXBULs zwwh4Z/NCYxYYhOZsuoUXKYbOz0UaOampLwKGHQU0RQdHdH+yaevg8nT4h/dgFp84UrS SoRGFkT4kqumMq6pmKJ2QhzUe4Gsh9C8d3HGz5FyGiRSdB8q55MGrBJwuRZrhexUCMtC 8sqfSWPENgBzC8F0gMw5AJ9ekoeJtAlD6/u/YE/8Y6zP/EyHPR1FMWheeKYaIpBfyV7h wWVY3eitJguMVO0iesYm2oiPHrutZcH+mwiRFY9RF642bm6z01NncoyA+ZaTtXlYnlDe XbNA== X-Gm-Message-State: AOJu0YyEldfGc9ruduK+NbT6F9qAmWtN659Ni9ztTvxDOUv0urG8Ip1a APEnigFsuTgYAbBfpZ6H1NsgLS95CcFc9B2TKBaED66Wha92gd5RTOsZfSge6ua6a2gNLxlgriw 1B29TgZ83t5Jpm5GIC1rktrenqXaqkb1cQan20hadNpgG1LIieQqmxw7S/g58UQ5slSc= X-Gm-Gg: ASbGncsROpT+wD+7w0MNNIQpK4/21vIsV0tgHakXldp/Nalt01vEu7fiwgXpvvTMPyX L2yyTy9zqo7kurOY2IK6vPisQvoCGW2VrGG8FeqWi2K8urJ8YwopUB6y3hwzjs3n6gDErGhCCWV a+7k6s7xav57uc3/4O2+VqhapjUXZZuyIxqFb5UXL+qoERpeWG62/uystsElHxiphXemk0Dl7Xn vAYDz9ZjzaeEjioq832zhRKU5s8tlLMzlYI1L0Bww4PnW7j4lN9n8xkeAN6GuZH7zI9zU+iL++6 CMrtAg== X-Received: by 2002:a05:600c:3ca0:b0:42a:a6d2:3270 with SMTP id 5b1f17b1804b1-436e26f01aamr219984645e9.21.1736936503748; Wed, 15 Jan 2025 02:21:43 -0800 (PST) X-Google-Smtp-Source: AGHT+IHr6aqkzDsn6Nqtk1QVK4nBMhNPOW5fBXXmo3OfajwNue39tZclhYVW67LGXXMv0Sm36SqAlA== X-Received: by 2002:a05:600c:3ca0:b0:42a:a6d2:3270 with SMTP id 5b1f17b1804b1-436e26f01aamr219984435e9.21.1736936503358; Wed, 15 Jan 2025 02:21:43 -0800 (PST) Received: from localhost (44.226.159.143.dyn.plus.net. [143.159.226.44]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-437c74d845esm17789115e9.27.2025.01.15.02.21.42 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 15 Jan 2025 02:21:43 -0800 (PST) From: Andrew Burgess To: Simon Marchi , Tom Tromey Cc: gdb-patches@sourceware.org, Guinevere Larsen Subject: Re: [PATCH] gdbserver: convert program_args to a single string In-Reply-To: References: <87ldvdqpgb.fsf@tromey.com> Date: Wed, 15 Jan 2025 10:21:42 +0000 Message-ID: <87frlkpebt.fsf@redhat.com> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: hUKpta4XfrdtIYfq3X6Gcdhm3R3708-OVvOETLbdHhc_1736936504 X-Mimecast-Originator: redhat.com Content-Type: text/plain X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gdb-patches-bounces~public-inbox=simark.ca@sourceware.org Simon Marchi writes: > On 2025-01-14 12:23, Tom Tromey wrote: >>>>>>> "Simon" == Simon Marchi writes: >> >>>> /* Start a new process. >>>> PROGRAM is the program name. >>>> - PROGRAM_ARGS is the vector containing the inferior's args. >>>> + PROGRAM_ARGS is a string containing all the inferior's arguments. >>>> Returns the new PID on success, -1 on failure. Registers the new >>>> process with the process list. */ >> >> Simon> I think this comment should just be removed, there's no point in >> Simon> repeating the documentation from the base class. >> >> FWIW I normally consider the "override" keyword to be sufficient >> documentation in these cases; though there's also some code with a >> comment pointing at the base class. > > Speaking of this, I don't really know what to do in these situations, I'd like > if we could clarify what our style is. Let's say you have: > > -- base.h > > struct base > { > /* 1 */ > virtual void method (); > }; > > -- base.c > > /* 2 */ > > void base::method () > { > } > > -- impl.c > > struct impl : public base > { > /* 3 */ > void method () override; > }; > > /* 4 */ > > void impl::method () > { > } > > Which comment should you have where? We typically say "all declarations > and definitions should have a comment", but I feel like it's unnecessary here. > > - #1 is where you should have the documentation about the behavior of > `method` As Tom said, the actual documentation could be at #1 or #2. With C++ implementing more stuff in headers, having the docs at #1 makes more and more sense I think. > - #2 would be the typical /* See base.h. */, although I find these > comments a bit useless It is a little useless, but I do like the consistency of all functions having something. And in some cases, when the declaration is not in the obvious place, it can help find where to look. > - As you said, I think the `override` keyword is sufficient to make > comment #3 unnecessary, unless you want to specify something specific > to `impl`. I don't see the need to point to the base class, that's > just implied by how the language works. Agreed. > - I never know what to put for #4, you can't put /* See impl.c. */, > since you're already in impl.c. Again, I feel like a "See whatever" > comment is a bit useless: if you know C++, you know you need to go > look at the declaration of struct/class `impl`. In cases where the base class declaration, and the out-of-line definitions are in the same file I write /* See class declaration. */ on the definitions (or something similar). In this example I'd probably go with /* See base.h. */ as that guides folk to where the comment can be found. Just thoughts. Thanks, Andrew