Re: [PATCH v2 7/7] convert.c: simplify text_stat

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

 



tboegi@xxxxxx writes:

>  static int convert_is_binary(unsigned long size, const struct text_stat *stats)
>  {
> -	if (stats->cr != stats->crlf)
> +	if (stats->lonecr)
>  		return 1;

This is an equivalent conversion, but...

> @@ -266,8 +267,8 @@ static int crlf_to_git(const char *path, const char *src, size_t len,
>  
>  	check_safe_crlf(path, crlf_action, &stats, checksafe);
>  
> -	/* Optimization: No CR? Nothing to convert, regardless. */
> -	if (!stats.cr)
> +	/* Optimization: No CRLF? Nothing to convert, regardless. */
> +	if (!stats.crlf)
>  		return 0;

... this is not.  In fact this is even better, as we used to try the
remainder of the function when we saw a lone CR, but we no longer
do.  I am of course assuming that the original turned into a no-op
if we had a lone CR in the input--otherwise this patch changes the
behaviour.

However, I see this comment after the function returns with the
above optimization:

       /*
        * If we guessed, we already know we rejected a file with
        * lone CR, and we can strip a CR without looking at what
        * follow it.
        */

So the code around there used to have a reason to worry about a lone
CR (i.e. it didn't want to lose them).  With the change in this hunk
for the "optimization", it no longer is necessary to do so, i.e. we
know we do not have a lone CR so every CR can be stripped because it
must be followed by LF, isn't it?

But I do not see a matching change to simplify that here.  Am I
following the current code incorrectly or something?

Puzzled...

>  	/*
> @@ -315,21 +316,16 @@ static int crlf_to_worktree(const char *path, const char *src, size_t len,
>  	gather_stats(src, len, &stats);
>  
>  	/* No LF? Nothing to convert, regardless. */
> -	if (!stats.lf)
> -		return 0;
> -
> -	/* Was it already in CRLF format? */
> -	if (stats.lf == stats.crlf)
> +	if (!stats.lonelf)

Doesn't the comment above need adjustment?

>  		return 0;

Otherwise, the output side looks correct to me.
--
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]