Re: [PATCH v2 06/18] report_refname_conflict(): inline function

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Michael Haggerty <mhagger@xxxxxxxxxxxx> writes:

> It wasn't pulling its weight. And we are about to need code similar to
> this where no ref_entry is available and with more diverse error
> messages. Rather than try to generalize the function, just inline it.
>
> Signed-off-by: Michael Haggerty <mhagger@xxxxxxxxxxxx>
> ---

I am not sure "not pulling its weight" in this and also 05/18 is a
fair judgement.  A short helper function sometimes helps readability
by giving an extra abstraction.

But I agree 100% with these two steps that they weren't making them
easier to read.  A raw call to error() shows clearly that we are
reporting an error and more importantly by losing an intermediary
we do not have to wonder at the call site if there is anything else
going on in that "report" function (where there isn't).

Thanks.


>  refs.c | 10 ++--------
>  1 file changed, 2 insertions(+), 8 deletions(-)
>
> diff --git a/refs.c b/refs.c
> index 6bdd34f..7422594 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -857,12 +857,6 @@ static int nonmatching_ref_fn(struct ref_entry *entry, void *vdata)
>  	return 1;
>  }
>  
> -static void report_refname_conflict(struct ref_entry *entry,
> -				    const char *refname)
> -{
> -	error("'%s' exists; cannot create '%s'", entry->name, refname);
> -}
> -
>  /*
>   * Return true iff a reference named refname could be created without
>   * conflicting with the name of an existing reference in dir.  If
> @@ -918,7 +912,7 @@ static int is_refname_available(const char *refname,
>  				 */
>  				return 1;
>  			}
> -			report_refname_conflict(entry, refname);
> +			error("'%s' exists; cannot create '%s'", entry->name, refname);
>  			return 0;
>  		}
>  
> @@ -969,7 +963,7 @@ static int is_refname_available(const char *refname,
>  		if (!do_for_each_entry_in_dir(dir, 0, nonmatching_ref_fn, &data))
>  			return 1;
>  
> -		report_refname_conflict(data.found, refname);
> +		error("'%s' exists; cannot create '%s'", data.found->name, refname);
>  		return 0;
>  	}
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]