Re: [RFC/PATCH] attr: Document a new possible thread safe API

From: Stefan Beller <sbeller@google.com>
Date: Oct 5, 2016, 17:04 UTC
Message-ID: <CAGZ79kZ7zNkXBVUFq5aLSup9xsdCbZpy_x8z5=uAQqftmOq26A@mail.gmail.com>
In-Reply-To: <xmqq8tu2ras4.fsf@gitster.mtv.corp.google.com>

On Wed, Oct 5, 2016 at 10:00 AM, Junio C Hamano <gitster@pobox.com> wrote:

Show 35 quoted lines

> Stefan Beller <sbeller@google.com> writes:
>
>> I thought about that, but as I concluded that the get_all_attrs doesn't need
>> conversion to a threading environment, we can keep it as is.
>
> I agree that it is OK for get_all_attrs() to use its own way to ask
> a question and receive an answer to it, that is different from how
> git_check_attr() asks its question.  The threading-support for it is
> an unrelated issue, though (not that I think it needs to be run from
> a multi-threaded caller).
>
>>> ...  I'd expect the most
>>> typical caller to be
>>>
>>>         static struct git_attr_check *check;
>>>         struct git_attr_result result[2]; /* we want two */
>>>
>>>         git_attr_check_initl(&check, "crlf", "ident", NULL);
>>>         git_check_attr(path, check, result);
>>>         /* result[0] has "crlf", result[1] has "ident" */
>>>
>>> or something like that.
>>
>> I see, that seems to be a clean API. So git_attr_check_initl
>> will lock the mutex and once it got the mutex it can either
>> * return early as someone else did the work
>> * needs to do the actual work
>> and then unlock. In any case the work was done.
>>
>> git_check_attr, which runs in all threads points to the same check,
>> but gets the different results.
>
> Yeah, something along that line.  It seems that we are now on the
> same page?
>

I think so, instead of resending the documentation, maybe the header file shows that we're on the same page, I converted everything except attr.c to follow this header attr.h:

---8<--- #ifndef ATTR_H #define ATTR_H

/* An attribute is a pointer to this opaque structure */ struct git_attr;

/*
 * Given a string, return the gitattribute object that
 * corresponds to it.
 */
extern struct git_attr *git_attr(const char *);

/*
 * Return the name of the attribute represented by the argument.  The
 * return value is a pointer to a null-delimited string that is part
 * of the internal data structure; it should not be modified or freed.
 */
extern const char *git_attr_name(const struct git_attr *);

extern int attr_name_valid(const char *name, size_t namelen); extern void invalid_attr_name_message(struct strbuf *, const char *, int);

/* Internal use */ extern const char git_attr__true[]; extern const char git_attr__false[];

/* For public to check git_attr_check results */ #define ATTR_TRUE(v) ((v) == git_attr__true) #define ATTR_FALSE(v) ((v) == git_attr__false) #define ATTR_UNSET(v) ((v) == NULL)

struct git_attr_check {
        int finalized;
        int check_nr;
        int check_alloc;
        struct git_attr **attr;
};

struct git_attr_result {
        int check_nr;
        int check_alloc;
        const char **value;
};

/*
 * Initialize the `git_attr_check` via one of the following three functions:
 *
 * git_attr_check_alloc allocates an empty check, add more attributes via
 *                      git_attr_check_append.
 * git_all_attrs        allocates a check and fills in all attributes that
 *                      are set for the given path.
 * git_attr_check_initl takes a pointer to where the check will be initialized,
 *                      followed by all attributes that are to be checked.
 *                      This makes it potentially thread safe as it could
 *                      internally have a mutex for that memory location.
 *                      Currently it is not thread safe!
 */
extern struct git_attr_check *git_attr_check_alloc(void);
extern void git_attr_check_append(struct git_attr_check *, const
struct git_attr *);
extern struct git_attr_check *git_all_attrs(const char *path);
extern void git_attr_check_initl(struct git_attr_check **, const char *, ...);

/* Query a path for its attributes */
extern struct git_attr_result *git_check_attr(const char *path,
                                             struct git_attr_check *);

/* internal? */ extern void git_attr_check_clear(struct git_attr_check *);

/* After use free the check */ extern void git_attr_check_free(struct git_attr_check *);

enum git_attr_direction {
        GIT_ATTR_CHECKIN,
        GIT_ATTR_CHECKOUT,
        GIT_ATTR_INDEX
};
void git_attr_set_direction(enum git_attr_direction, struct index_state *);

#endif /* ATTR_H */