Re: [PATCH 08/67] add reentrant variants of sha1_to_hex and find_unique_abbrev
- From
Jeff King <peff@peff.net>
- Date
- Sep 16, 2015, 17:23 UTC
- Message-ID
- <20150916172347.GA2234@sigill.intra.peff.net>
- In-Reply-To
- <xmqq613as399.fsf@gitster.mtv.corp.google.com>
On Wed, Sep 16, 2015 at 10:06:10AM -0700, Junio C Hamano wrote:
Show 19 quoted lines
> Jeff King <peff@peff.net> writes: > > > On Wed, Sep 16, 2015 at 10:15:02AM +0200, Johannes Schindelin wrote: > > > >> Maybe we should stick to the established practice of the many, many > >> reentrant POSIX functions following the *_r() naming convention? I.e. > >> the reentrant version of localtime() is called localtime_r(), the > >> reentrant version of random() is called random_r() etc. > >> > >> So I could see myself not needing an explanation if I had read > >> sha1_to_hex_r(...). > > > > I like this suggestion. By itself, the "_r" does not communicate as much > > as "_to" to me, but as long as the reader knows the "_r" idiom, it > > communicates much more. > > > > I'll switch to this unless there is any objection. > > Fine by me. Thanks.
I started on this, but realized something interesting as I was updating the docstrings. sha1_to_hex_r is truly reentrant. But find_unique_abbrev_r is not, as it calls has_sha1_file(), which touches lots of global data for the object lookup.
I think it's probably OK, as long as I don't make the claim that it is truly reentrant.
-Peff