From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 18922 invoked by alias); 11 Sep 2014 17:12:12 -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 18911 invoked by uid 89); 11 Sep 2014 17:12:10 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-5.1 required=5.0 tests=BAYES_00,RCVD_IN_DNSWL_LOW,RP_MATCHES_RCVD,SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-vc0-f176.google.com Received: from mail-vc0-f176.google.com (HELO mail-vc0-f176.google.com) (209.85.220.176) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Thu, 11 Sep 2014 17:12:05 +0000 Received: by mail-vc0-f176.google.com with SMTP id la4so4431100vcb.35 for ; Thu, 11 Sep 2014 10:12:03 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:date :message-id:subject:from:to:cc:content-type; bh=ZmTlDxt6ahl4Wq3g0fGvuxugfE0pml8oZaty2LGsUEg=; b=LR9nhZOxwcocv8D0YF8YsEiJruCawYgTYlQPmPfLDcYxvvhTvinKdMDvJEVu245vfL 7CnjO9oi9OpZ3Pfgg0UuaAux9YsfUpUtc8yv3XHSsNhVrlzDUhjG0Njsmy5fYLA6r6wd vMz6DCP1xsSYaQamlbxUDkmEFQtMhqbhl4w1ovOFBO1N+WWrs9O+KWGHvX1Cm6tuJRQT QekCBynWmOQVzV/Ku+bk5cCfNCR1/3WDzhqoN5PbR6oH8gyD5YvdVihvO324vJI7c4Hr MafDFi7s6ys2QVAKrWd8I6jqpiv2k/u/DtLSyLVyWAZh+xy7lgjxA1V9nOnFfn/hKEwy z4Lg== X-Gm-Message-State: ALoCoQmFnheVRZfrMjtdvU48WdNxQGyF/yREZGzG6WmvysC7tJJwgnZp+aRTRjqUEXKX3jre0Oni MIME-Version: 1.0 X-Received: by 10.52.240.135 with SMTP id wa7mr1367715vdc.82.1410455523609; Thu, 11 Sep 2014 10:12:03 -0700 (PDT) Received: by 10.52.136.203 with HTTP; Thu, 11 Sep 2014 10:12:03 -0700 (PDT) In-Reply-To: <20140911110231.GC17472@blade.nx> References: <1409320299-6812-1-git-send-email-gbenson@redhat.com> <1409320299-6812-6-git-send-email-gbenson@redhat.com> <21520.37315.16694.677898@ruffy2.mtv.corp.google.com> <20140911110231.GC17472@blade.nx> Date: Thu, 11 Sep 2014 17:12:00 -0000 Message-ID: Subject: Re: [PATCH 5/9 v7] Introduce common-regcache.h From: Doug Evans To: Gary Benson Cc: gdb-patches , Pedro Alves Content-Type: text/plain; charset=UTF-8 X-IsSubscribed: yes X-SW-Source: 2014-09/txt/msg00364.txt.bz2 On Thu, Sep 11, 2014 at 4:02 AM, Gary Benson wrote: > Doug Evans wrote: >> Gary Benson writes: >> > +/* Return the register cache associated with the thread specified >> > + by PTID. This function must be provided by the client. */ >> >> Can you add a comment here explaining the ownership of the memory >> object returned? E.g., is it cached "internally" to the function >> so that the caller doesn't have to free it? > > That seems odd. We don't document other similar functions this way: > I'm thinking GDB's get_current_arch, current_inferior, target_gdbarch, > or gdbserver's current_process, current_target_desc. It seems the > pattern is to note if the caller must free the object and to remain > quiet otherwise. Yeah, but I never liked such things being implicit. I can't trust a missing "caller must free" as being intentional. [One can equally argue the presence of "caller must free" (or "not free") isn't necessarily trustable, but such things don't change that often.] With a name like "get_current_foo", the "current" makes things less implicit (at least to me). If we were using c++ then object ownership can (often, though not always) be more clearly expressed and then such things can be more reasonably left as being implicit in comments. But we don't have that so if we're going to be cleaning things up, and maybe even paying a little attention to API design, I figure IWBN to have things be clearer than they are today. [Plus I get bitten time and again by taking gdb's existing practice as something we actually want to keep. :-)] > How about I change the comment to "Return _a_pointer_to_ the register > cache..."? (underlines for emphasis here). If one was going to add emphasis, I'd emphasize _the_. :-)