From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on dcvr.yhbt.net X-Spam-Level: X-Spam-Status: No, score=-3.7 required=3.0 tests=AWL,BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,RCVD_IN_DNSWL_MED, RCVD_IN_MSPIKE_H4,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_PASS shortcircuit=no autolearn=ham autolearn_force=no version=3.4.2 Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dcvr.yhbt.net (Postfix) with ESMTPS id 0255B1F4B4 for ; Sun, 13 Sep 2020 12:11:43 +0000 (UTC) Received: from localhost ([::1]:48074 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1kHQr8-0002JL-Sl for normalperson@yhbt.net; Sun, 13 Sep 2020 08:11:42 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:45690) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1kHQr1-0002Iy-2h for bug-gnulib@gnu.org; Sun, 13 Sep 2020 08:11:35 -0400 Received: from de.cellform.com ([88.217.224.109]:37894 helo=jocasta.intra) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1kHQqy-00071x-PJ for bug-gnulib@gnu.org; Sun, 13 Sep 2020 08:11:34 -0400 Received: from jocasta.intra (localhost [127.0.0.1]) by jocasta.intra (8.15.2/8.15.2/Debian-14~deb10u1) with ESMTPS id 08DCBPuN021607 (version=TLSv1.3 cipher=TLS_AES_256_GCM_SHA384 bits=256 verify=NOT); Sun, 13 Sep 2020 14:11:26 +0200 Received: (from john@localhost) by jocasta.intra (8.15.2/8.15.2/Submit) id 08DCBP8v021606; Sun, 13 Sep 2020 14:11:25 +0200 Date: Sun, 13 Sep 2020 14:11:25 +0200 From: John Darrington To: Bruno Haible Subject: Re: [PATCH] tmpdir.c (path_search_alloc): New function. Message-ID: <20200913121125.GA21210@jocasta.intra> References: <20200913093334.17844-1-john@darrington.wattle.id.au> <3111347.7ZDk9rG9Yt@omega> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <3111347.7ZDk9rG9Yt@omega> User-Agent: Mutt/1.10.1 (2018-07-13) Received-SPF: pass client-ip=88.217.224.109; envelope-from=john@darrington.wattle.id.au; helo=jocasta.intra X-detected-operating-system: by eggs.gnu.org: First seen = 2020/09/13 08:11:29 X-ACL-Warn: Detected OS = Linux 3.11 and newer [fuzzy] X-Spam_score_int: -18 X-Spam_score: -1.9 X-Spam_bar: - X-Spam_report: (-1.9 / 5.0 requ) BAYES_00=-1.9, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: bug-gnulib@gnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Gnulib discussion list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: bug-gnulib@gnu.org Errors-To: bug-gnulib-bounces+normalperson=yhbt.net@gnu.org Sender: "bug-gnulib" On Sun, Sep 13, 2020 at 01:12:28PM +0200, Bruno Haible wrote: Hi John, As this is your first major contribution to Gnulib, can you please make sure you have signed a copyright assignment with the FSF regarding 'GNULIB'? Yes, I have. > * lib/tmpdir.c (path_search_alloc): Define new function similar to > path_search, but which allocates the buffer for the result instead > of relying on the caller to preallocate it. This is a good idea, because it gets rid of an EINVAL error return. It also means the caller does not have to guess how long the buffer should be. > +/* Like path_search, except that TMPL is allocated automatically. > + TMPL may not be null. *TMPL must be freed by the caller, when no longer needed. > + After calling this function *TMPL_LEN will be set to the lenght of *TMPL. */ > +extern int path_search_alloc (char **tmpl, size_t *tmpl_len, const char *dir, const char *pfx, > + bool try_tmpdir); The calling convention is odd: If the caller is only meant to use *TMPL and later free() it, why does he need *TMPL_LEN? It seems redundant to return it from this function. And then, if *TMPL is the only output (besides the error condition), why not make it the return value? That is: extern char * path_search_alloc (const char *dir, const char *pfx, bool try_tmpdir); In case of error, this function would return NULL with errno set. That would also work. But I don't think the suggested interface is particularly odd. It is very similar to the getline function from libc. Regarding TMPL_LEN, you are right, it *may* not be needed. However I often find that when writing code which munges strings, one needs to know the length of a string which has already been calculated. Of course one could simply use strlen to find it, but strlen is O(n) So I think, that if a function has to calculate a length of a string anyway, then it is worthwhile making that number available to the caller. The caller is free to use it or ignore it as she wishes. Often she'll be gratefull for it. Typo: s/lenght/length/ Thanks for noticing. Please don't choose function names that start with underscore. These function names are in the scope of the vendor libc. And in fact, such function names would make merging with glibc harder, since glibc already has a function '__path_search' (defined in sysdeps/posix/tempname.c). I wondered why that was there. Also, __path_search is a misnomer now: it does not search anything; it determines the temporary directory in which to place a temporary file. So what name would you suggest? "get_temp_directory" ? J'