On Sun, Dec 6, 2015 at 3:38 PM, Torsten Bögershausen <tboegi@xxxxxx> wrote:
> When working in a cross-platform environment, a user wants to
> check if text files are stored normalized in the repository and if
> .gitattributes are set appropriately.
> [...]
> Signed-off-by: Torsten Bögershausen <tboegi@xxxxxx>
> ---
> diff --git a/convert.c b/convert.c
> @@ -95,6 +100,63 @@ static int is_binary(unsigned long size, struct text_stat *stats)
> +const char *get_wt_convert_stats_ascii(const char *path)
> +{
> + const char *ret;
> + struct strbuf sb = STRBUF_INIT;
> + if (strbuf_read_file(&sb, path, 0) < 0)
> + return "";
Given the implementation of strbuf_read_file(), some memory may have
been allocated to the strbuf even if it returns a failure, therefore
this may be leaking.
> + ret = gather_convert_stats_ascii(sb.buf, sb.len);
> + strbuf_release(&sb);
> + return ret;
> +}
You may, therefore, want to restructure the code like this to avoid the leak:
const char *ret = "";
struct strbuf sb = STRBUF_INIT;
if (strbuf_read_file(...) >= 0)
ret = gather_convert_stats_ascii(...);
strbuf_release(&sb);
return ret;
> diff --git a/t/t0027-auto-crlf.sh b/t/t0027-auto-crlf.sh
> @@ -213,7 +238,20 @@ checkout_files () {
> git -c core.eol=$eol checkout $src$f.txt
> fi
> done
> -
Was dropping this blank line intentional?
> + test_expect_success "ls-files --eol $lfname ${pfx}LF.txt" "
> + cat >e <<-EOF &&
> + i/text-crlf w/$(stats_ascii $crlfname) ${src}CRLF.txt
> + i/text-crlf-lf w/$(stats_ascii $lfmixcrlf) ${src}CRLF_mix_LF.txt
> + i/text-lf w/$(stats_ascii $lfname) ${src}LF.txt
> + i/binary w/$(stats_ascii $lfmixcr) ${src}LF_mix_CR.txt
> + i/binary w/$(stats_ascii $crlfnul) ${src}CRLF_nul.txt
> + i/binary w/$(stats_ascii $crlfnul) ${src}LF_nul.txt
> + EOF
> + sort <e >expect &&
> + git ls-files --eol $src* | sed -e 's!attr/[=a-z-]*!!g' -e 's/ */ /g' | sort >actual &&
> + test_cmp expect actual &&
> + rm e expect actual
If it is indeed important to cleanup these files so that they don't
muck up following tests, then you'd be better off using:
test_when_finished "rm -f e expect actual"
at the top of the test to ensure that the cleanup happens regardless
of the whether the test succeeds or fails. On the other hand, if these
files won't muck up subsequent tests, then it may be fine to skip
cleanup altogether.
> + "
> test_expect_success "checkout core.eol=$eol core.autocrlf=$crlf gitattributes=$attr file=LF" "
> compare_ws_file $pfx $lfname ${src}LF.txt
> "
> @@ -231,6 +269,37 @@ checkout_files () {
> +# Test control characters
> +# NUL SOH CR EOF==^Z
> +test_expect_success 'ls-files --eol -o Text/Binary' '
> + test_when_finished "rm e expect actual TeBi_*" &&
It's probably not a big deal, but "rm -f" perhaps?
> + STRT=AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA &&
> + STR=$STRT$STRT$STRT$STRT &&
> + printf "${STR}BBB\001" >TeBi_127_S &&
> + printf "${STR}BBBB\001">TeBi_128_S &&
> + printf "${STR}BBB\032" >TeBi_127_E &&
> + printf "\032${STR}BBB" >TeBi_E_127 &&
> + printf "${STR}BBBB\000">TeBi_128_N &&
> + printf "${STR}BBB\012">TeBi_128_L &&
> + printf "${STR}BBB\015">TeBi_127_C &&
> + printf "${STR}BB\015\012" >TeBi_126_CL &&
> + printf "${STR}BB\015\012\015" >TeBi_126_CLC &&
> + cat >e <<-\EOF &&
> + i/ w/binary TeBi_127_S
> + i/ w/text-no-eol TeBi_128_S
> + i/ w/text-no-eol TeBi_127_E
> + i/ w/binary TeBi_E_127
> + i/ w/binary TeBi_128_N
> + i/ w/text-lf TeBi_128_L
> + i/ w/binary TeBi_127_C
> + i/ w/text-crlf TeBi_126_CL
> + i/ w/binary TeBi_126_CLC
> + EOF
> + sort <e >expect &&
> + git ls-files --eol -o | egrep "TeBi_" | sed -e 's!attr/[=a-z-]*!!g' -e "s/ */ /g" | sort >actual &&
Okay. "egrep" is used about as frequently in test scripts as "grep -E".
> + test_cmp expect actual
> +'
> @@ -480,4 +549,20 @@ checkout_files native true "lf" LF CRLF CRLF_mix_LF LF_mix_CR
> +# Should be the last test case
> +test_expect_success 'ls-files --eol -d' "
> + rm crlf_false_attr__CRLF.txt crlf_false_attr__CRLF_mix_LF.txt crlf_false_attr__LF.txt .gitattributes &&
Did you mean to wrap this in test_when_finished()?
If not, is it cleaning up gunk left by some earlier test(s)? If so,
what happens if one of those tests failed and didn't create one of
these files? In that case, this 'rm' would fail, causing this entire
test to fail. Better would be to use "rm -f".
> + cat >expect <<-\EOF &&
> + i/text-crlf w/ crlf_false_attr__CRLF.txt
> + i/text-crlf-lf w/ crlf_false_attr__CRLF_mix_LF.txt
> + i/text-lf w/ .gitattributes
> + i/text-lf w/ crlf_false_attr__LF.txt
> + EOF
> + git ls-files --eol -d | sed -e 's!attr/[=a-z-]*!!g' -e 's/ */ /g' | sort >actual &&
> + test_cmp expect actual &&
> + rm expect actual
> +"
> +
> +
Style: Drop the extra blank line.
> test_done
> --
> 2.6.2.403.gd7a84e3
--
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