Re: [PATCH v6 05/32] refs: add a backend method structure with transaction functions

> On Wed, Feb 24, 2016 at 05:58:37PM -0500, David Turner wrote:
> 
> > +int set_ref_storage_backend(const char *name)
> > +{
> > +	struct ref_storage_be *be;
> > +
> > +	for (be = refs_backends; be; be = be->next)
> > +		if (!strcmp(be->name, name)) {
> > +			the_refs_backend = be;
> > +			return 0;
> > +		}
> > +	return 1;
> > +}
> 
> So we search through the list and assign the_refs_backend if we find
> something, returning 0 for success, and 1 if we found nothing. OK
> (though our usual convention is that if 0 is success, -1 is the error
> case).

> > +int ref_storage_backend_exists(const char *name)
> > +{
> > +	struct ref_storage_be *be;
> > +
> > +	for (be = refs_backends; be; be = be->next)
> > +		if (!strcmp(be->name, name)) {
> > +			the_refs_backend = be;
> > +			return 1;
> > +		}
> > +	return 0;
> > +}
> 
> Here we do the same thing, but we get "1" for exists, and "0"
> otherwise. That makes sense if this is about querying for existence.
> But
> why does the query function still set the_refs_backend?

> I'm guessing the assignment in the second one is just copy-pasta, but
> maybe the whole thing would be simpler if they could share the
> implementation, like:
> 
>   struct ref_storage_be *find_ref_storage_backend(const char *name)
>   {
> 	struct ref_storage *be;
> 	for (be = refs_backends; be; be = be->next)
> 		if (!strcmp(be->name, name))
> 			return be;
> 	return NULL;
>   }
> 
>   int set_ref_storage_backend(const char *name)
>   {
> 	struct ref_storage *be = find_ref_storage_backend(name);
> 	if (!be)
> 		return -1;
> 	the_refs_backend = be;
> 	return 0;
>   }
> 
> You don't really need an "exists", as you can check that "find"
> doesn't
> return NULL, but you could wrap it, of course.