Re: [PATCH v9 5/8] convert: add 'working-tree-encoding' attribute
- From
Eric Sunshine <sunshine@sunshineco.com>
- Date
- Mar 6, 2018, 20:42 UTC
- Message-ID
- <CAPig+cTOpBODeoHV=+4-4MEjKM=pUrZSa=BEmHh4mVP=xPpFCA@mail.gmail.com>
- In-Reply-To
- <20180304201418.60958-6-lars.schneider@autodesk.com>
On Sun, Mar 4, 2018 at 3:14 PM, <lars.schneider@autodesk.com> wrote:
Show 18 quoted lines
> Git recognizes files encoded with ASCII or one of its supersets (e.g.
> UTF-8 or ISO-8859-1) as text files. All other encodings are usually
> interpreted as binary and consequently built-in Git text processing
> tools (e.g. 'git diff') as well as most Git web front ends do not
> visualize the content.
> [...]
> Signed-off-by: Lars Schneider <larsxschneider@gmail.com>
> ---
> diff --git a/convert.c b/convert.c
> @@ -978,6 +1051,25 @@ static int ident_to_worktree(const char *path, const char *src, size_t len,
> +static const char *git_path_check_encoding(struct attr_check_item *check)
> +{
> + [...]
> + /*
> + * Ensure encoding names are always upper case (e.g. UTF-8) to
> + * simplify subsequent string comparisons.
> + */
> + return xstrdup_toupper(value);xstrdup_toupper() allocates memory...
Show 6 quoted lines
> +}
> @@ -1033,6 +1125,7 @@ struct conv_attrs {
> enum crlf_action attr_action; /* What attr says */
> enum crlf_action crlf_action; /* When no attr is set, use core.autocrlf */
> int ident;
> + const char *working_tree_encoding; /* Supported encoding or default encoding if NULL */...which is assigned to 'const char *'...
Show 6 quoted lines
> }; > @@ -1064,6 +1158,7 @@ static void convert_attrs(struct conv_attrs *ca, const char *path) > else if (eol_attr == EOL_CRLF) > ca->crlf_action = CRLF_TEXT_CRLF; > } > + ca->working_tree_encoding = git_path_check_encoding(ccheck + 5);
...by this code, and eventually leaked.
It's too bad it isn't cleaned up (freed), but looking at the callers, fixing this leak would be mildly noisy (though not particularly invasive). How much do we care about this leak?
Show 7 quoted lines
> } else {
> ca->drv = NULL;
> ca->crlf_action = CRLF_UNDEFINED;
> diff --git a/t/t0028-working-tree-encoding.sh b/t/t0028-working-tree-encoding.sh
> @@ -0,0 +1,135 @@
> +test_expect_success 'check $GIT_DIR/info/attributes support' '
> + test_when_finished "rm -f test.utf8.raw test.utf32.raw test.utf32.git" &&It seems weird to be cleaning up files this test didn't create (test.utf8.raw and test.utf32.raw).
Show 12 quoted lines
> + test_when_finished "git reset --hard HEAD" && > + > + echo "*.utf32 text working-tree-encoding=utf-32" >.git/info/attributes && > + git add test.utf32 && > + > + git cat-file -p :test.utf32 >test.utf32.git && > + test_cmp_bin test.utf8.raw test.utf32.git > +' > + > +test_expect_success 'check unsupported encodings' ' > + test_when_finished "rm -f err.out" && > + test_when_finished "git reset --hard HEAD" &&
Resetting to HEAD here is an important cleanup action, but tests don't usually clean up files such as 'err.out' since such detritus doesn't usually impact subsequent tests negatively. (Just an observation; no re-roll needed.)
Show 9 quoted lines
> + echo "*.nothing text working-tree-encoding=" >>.gitattributes && > + printf "nothing" >t.nothing && > + git add t.nothing && > + > + echo "*.garbage text working-tree-encoding=garbage" >>.gitattributes && > + printf "garbage" >t.garbage && > + test_must_fail git add t.garbage 2>err.out && > + test_i18ngrep "fatal: failed to encode" err.out > +'