Re: [PATCH 27/36] attr: convert to new threadsafe API

From: Duy Nguyen <pclouds@gmail.com>
Date: Oct 26, 2016, 13:52 UTC
Message-ID: <CACsJy8AB-24dDEs6Pa_6pB16T2ZAgw2faOQDEtZ2vU2uuAepHg@mail.gmail.com>
In-Reply-To: <20161022233225.8883-28-sbeller@google.com>

On Sun, Oct 23, 2016 at 6:32 AM, Stefan Beller <sbeller@google.com> wrote:

Show 8 quoted lines

> This revamps the API of the attr subsystem to be thread safe.
> Before we had the question and its results in one struct type.
> The typical usage of the API was
>
>     static struct git_attr_check *check;
>
>     if (!check)
>         check = git_attr_check_initl("text", NULL);

Two cents. I read the .txt first and my first thought was "is _initl a typo, shouldn't it be jsut _init"? I know we have this 'l' variant at least in argv-array, but there we have many ways of adding arguments. And here it's just "initl", not "init" nor other "initX" variants, which looks odd. I wonder if the name git_attr_check_init() would do the job fine, since we don't have different init variants and the naming convention is not strong enough to tell me "it's multiple arguments ended with a NULL one" right away. If you're worried about people forgetting NULL at the end, how about passing an array of strings, terminated by NULL, instead?

Just thinking out loud. Maybe _initl _is_ a better name.

-- 
Duy