Commit Graph

314 Commits

Author SHA1 Message Date
Junio C Hamano
feba8be3f0 Merge branch 'rj/ref-filter-get-head-description-leakfix' into maint-2.38
Leakfix.

* rj/ref-filter-get-head-description-leakfix:
  ref-filter.c: fix a leak in get_head_description
2022-10-27 15:24:11 -07:00
Rubén Justo
abcac2e19f ref-filter.c: fix a leak in get_head_description
In 2708ce62d2 (branch: sort detached HEAD based on a flag, 2021-01-07) a
call to wt_status_state_free_buffers, responsible of freeing the
resources that could be allocated in the local struct wt_status_state
state, was eliminated.

The call to wt_status_state_free_buffers was introduced in 962dd7ebc3
(wt-status: introduce wt_status_state_free_buffers(), 2020-09-27).  This
commit brings back that call in get_head_description.

Signed-off-by: Rubén Justo <rjusto@gmail.com>
Reviewed-by: Martin Ågren <martin.agren@gmail.com>
Acked-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-09-26 11:14:49 -07:00
Ævar Arnfjörð Bjarmason
5cf88fd8b0 git-compat-util.h: use "UNUSED", not "UNUSED(var)"
As reported in [1] the "UNUSED(var)" macro introduced in
2174b8c75de (Merge branch 'jk/unused-annotation' into next,
2022-08-24) breaks coccinelle's parsing of our sources in files where
it occurs.

Let's instead partially go with the approach suggested in [2] of
making this not take an argument. As noted in [1] "coccinelle" will
ignore such tokens in argument lists that it doesn't know about, and
it's less of a surprise to syntax highlighters.

This undoes the "help us notice when a parameter marked as unused is
actually use" part of 9b24034754 (git-compat-util: add UNUSED macro,
2022-08-19), a subsequent commit will further tweak the macro to
implement a replacement for that functionality.

1. https://lore.kernel.org/git/220825.86ilmg4mil.gmgdl@evledraar.gmail.com/
2. https://lore.kernel.org/git/220819.868rnk54ju.gmgdl@evledraar.gmail.com/

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-09-01 10:49:48 -07:00
Jeff King
02c3c59e62 hashmap: mark unused callback parameters
Hashmap comparison functions must conform to a particular callback
interface, but many don't use all of their parameters. Especially the
void cmp_data pointer, but some do not use keydata either (because they
can easily form a full struct to pass when doing lookups). Let's mark
these to make -Wunused-parameter happy.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-08-19 12:18:55 -07:00
Jeff King
359b01ca84 ref-filter: disable save_commit_buffer while traversing
Various ref-filter options like "--contains" or "--merged" may cause us
to traverse large segments of the history graph. It's counter-productive
to have save_commit_buffer turned on, as that will instruct the commit
code to cache in-memory the object contents for each commit we traverse.

This increases the amount of heap memory used while providing little or
no benefit, since we're not actually planning to display those commits
(which is the usual reason that tools like git-log want to keep them
around). We can easily disable this feature while ref-filter is running.
This lowers peak heap (as measured by massif) for running:

  git tag --contains 1da177e4c3

in linux.git from ~100MB to ~20MB. It also seems to improve runtime by
4-5% (600ms vs 630ms).

A few points to note:

  - it should be safe to temporarily disable save_commit_buffer like
    this. The saved buffers are accessed through get_commit_buffer(),
    which treats the saved ones like a cache, and loads on-demand from
    the object database on a cache miss. So any code that was using this
    would not be wrong, it might just incur an extra object lookup for
    some objects. But...

  - I don't think any ref-filter related code is using the cache. While
    it's true that an option like "--format=%(*contents:subject)" or
    "--sort=*authordate" will need to look at the commit contents,
    ref-filter doesn't use get_commit_buffer() to do so! It always reads
    the objects directly via read_object_file(), though it does avoid
    re-reading objects if the format can be satisfied without them.

    Timing "git tag --format=%(*authordate)" shows that we're the same
    before and after, as expected.

  - Note that all of this assumes you don't have a commit-graph file. if
    you do, then the heap usage is even lower, and the runtime is 10x
    faster. So in that sense this is not urgent, as there's a much
    better solution. But since it's such an obvious and easy win for
    fallback cases (including commits which aren't yet in the graph
    file), there's no reason not to.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-07-11 14:27:31 -07:00
Junio C Hamano
2da81d1efb Merge branch 'ab/plug-leak-in-revisions'
Plug the memory leaks from the trickiest API of all, the revision
walker.

* ab/plug-leak-in-revisions: (27 commits)
  revisions API: add a TODO for diff_free(&revs->diffopt)
  revisions API: have release_revisions() release "topo_walk_info"
  revisions API: have release_revisions() release "date_mode"
  revisions API: call diff_free(&revs->pruning) in revisions_release()
  revisions API: release "reflog_info" in release revisions()
  revisions API: clear "boundary_commits" in release_revisions()
  revisions API: have release_revisions() release "prune_data"
  revisions API: have release_revisions() release "grep_filter"
  revisions API: have release_revisions() release "filter"
  revisions API: have release_revisions() release "cmdline"
  revisions API: have release_revisions() release "mailmap"
  revisions API: have release_revisions() release "commits"
  revisions API users: use release_revisions() for "prune_data" users
  revisions API users: use release_revisions() with UNLEAK()
  revisions API users: use release_revisions() in builtin/log.c
  revisions API users: use release_revisions() in http-push.c
  revisions API users: add "goto cleanup" for release_revisions()
  stash: always have the owner of "stash_info" free it
  revisions API users: use release_revisions() needing REV_INFO_INIT
  revision.[ch]: document and move code declared around "init"
  ...
2022-06-07 14:10:56 -07:00
Junio C Hamano
538dc459a0 Merge branch 'ep/maint-equals-null-cocci'
Introduce and apply coccinelle rule to discourage an explicit
comparison between a pointer and NULL, and applies the clean-up to
the maintenance track.

* ep/maint-equals-null-cocci:
  tree-wide: apply equals-null.cocci
  tree-wide: apply equals-null.cocci
  contrib/coccinnelle: add equals-null.cocci
2022-05-20 15:26:59 -07:00
Junio C Hamano
2b0a58d164 Merge branch 'ep/maint-equals-null-cocci' for maint-2.35
* ep/maint-equals-null-cocci:
  tree-wide: apply equals-null.cocci
  contrib/coccinnelle: add equals-null.cocci
2022-05-02 10:06:04 -07:00
Junio C Hamano
afe8a9070b tree-wide: apply equals-null.cocci
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-05-02 09:50:37 -07:00
Ævar Arnfjörð Bjarmason
2108fe4a19 revisions API users: add straightforward release_revisions()
Add a release_revisions() to various users of "struct rev_list" in
those straightforward cases where we only need to add the
release_revisions() call to the end of a block, and don't need to
e.g. refactor anything to use a "goto cleanup" pattern.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-04-13 23:56:08 -07:00
Ævar Arnfjörð Bjarmason
974c919d36 date API: add and use a date_mode_release()
Fix a memory leak in the parse_date_format() function by providing a
new date_mode_release() companion function.

By using this in "t/helper/test-date.c" we can mark the
"t0006-date.sh" test as passing when git is compiled with
SANITIZE=leak, and whitelist it to run under
"GIT_TEST_PASSING_SANITIZE_LEAK=true" by adding
"TEST_PASSES_SANITIZE_LEAK=true" to the test itself.

The other tests that expose this memory leak (i.e. take the
"mode->type == DATE_STRFTIME" branch in parse_date_format()) are
"t6300-for-each-ref.sh" and "t7004-tag.sh". The former is due to an
easily fixed leak in "ref-filter.c", and brings the failures in
"t6300-for-each-ref.sh" down from 51 to 48.

Fixing the remaining leaks will have to wait until there's a
release_revisions() in "revision.c", as they have to do with leaks via
"struct rev_info".

There is also a leak in "builtin/blame.c" due to its call to
parse_date_format() to parse the "blame.date" configuration. However
as it declares a file-level "static struct date_mode blame_date_mode"
to track the data, LSAN will not report it as a leak. It's possible to
get valgrind(1) to complain about it with e.g.:

    valgrind --leak-check=full --show-leak-kinds=all ./git -P -c blame.date=format:%Y blame README.md

But let's focus on things LSAN complains about, and are thus
observable with "TEST_PASSES_SANITIZE_LEAK=true". We should get to
fixing memory leaks in "builtin/blame.c", but as doing so would
require some re-arrangement of cmd_blame() let's leave it for some
other time.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-02-16 09:40:00 -08:00
Ævar Arnfjörð Bjarmason
f184289832 date API: provide and use a DATE_MODE_INIT
Provide and use a DATE_MODE_INIT macro. Most of the users of struct
date_mode" use it via pretty.h's "struct pretty_print_context" which
doesn't have an initialization macro, so we're still bound to being
initialized to "{ 0 }" by default.

But we can change the couple of callers that directly declared a
variable on the stack to instead use the initializer, and thus do away
with the "mode.local = 0" added in add00ba2de (date: make "local"
orthogonal to date format, 2015-09-03).

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-02-16 09:40:00 -08:00
Jean-Noël Avila
d7d30badbf i18n: ref-filter: factorize "%(foo) atom used without %(bar) atom"
Signed-off-by: Jean-Noël Avila <jn.avila@free.fr>
Reviewed-by: Johannes Sixt <j6t@kdbg.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-01-05 13:31:00 -08:00
Jean-Noël Avila
68e2ea0b30 i18n: refactor "unrecognized %(foo) argument" strings
Signed-off-by: Jean-Noël Avila <jn.avila@free.fr>
Reviewed-by: Johannes Sixt <j6t@kdbg.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-01-05 13:31:00 -08:00
Junio C Hamano
5126145ba8 Merge branch 'jc/fix-ref-sorting-parse'
Things like "git -c branch.sort=bogus branch new HEAD", i.e. the
operation modes of the "git branch" command that do not need the
sort key information, no longer errors out by seeing a bogus sort
key.

* jc/fix-ref-sorting-parse:
  for-each-ref: delay parsing of --sort=<atom> options
2021-11-29 15:41:47 -08:00
Jiang Xin
f733719316 i18n: fix typos found during l10n for git 2.34.0
Emir and Jean-Noël reported typos in some i18n messages when preparing
l10n for git 2.34.0.

* Fix unstable spelling of config variable "gpg.ssh.defaultKeyCommand"
  which was introduced in commit fd9e226776 (ssh signing: retrieve a
  default key from ssh-agent, 2021-09-10).

* Add missing space between "with" and "--python" which was introduced
  in commit bd0708c7eb (ref-filter: add %(raw) atom, 2021-07-26).

* Fix unmatched single quote in 'builtin/index-pack.c' which was
  introduced in commit 8737dab346 (index-pack: refactor renaming in
  final(), 2021-09-09)

[1] https://github.com/git-l10n/git-po/pull/567

Reported-by: Emir Sarı <bitigchi@me.com>
Reported-by: Jean-Noël Avila <jn.avila@free.fr>
Signed-off-by: Jiang Xin <worldhello.net@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-10-31 22:49:49 -07:00
Junio C Hamano
98e7ab6d42 for-each-ref: delay parsing of --sort=<atom> options
The for-each-ref family of commands invoke parsers immediately when
it sees each --sort=<atom> option, and die before even seeing the
other options on the command line when the <atom> is unrecognised.

Instead, accumulate them in a string list, and have them parsed into
a ref_sorting structure after the command line parsing is done.  As
a consequence, "git branch --sort=bogus -h" used to fail to give the
brief help, which arguably may have been a feature, now does so,
which is more consistent with how other options work.

The patch is smaller than the actual extent of the "damage" to the
codebase, thanks to the fact that the original code consistently
used OPT_REF_SORT() macro to handle command line options.  We only
needed to replace the variable used for the list, and implementation
of the callback function used in the macro.

The old rule was for the users of the API to:

 - Declare ref_sorting and ref_sorting_tail variables;

 - OPT_REF_SORT() macro will instantiate ref_sorting instance (which
   may barf and die) and append it to the tail;

 - Append to the tail each ref_sorting read from the configuration
   by parsing in the config callback (which may barf and die);

 - See if ref_sorting is null and use ref_sorting_default() instead.

Now the rule is not all that different but is simpler:

 - Declare ref_sorting_options string list.

 - OPT_REF_SORT() macro will append it to the string list;

 - Append to the string list the sort key read from the
   configuration;

 - call ref_sorting_options() to turn the string list to ref_sorting
   structure (which also deals with the default value).

As side effects, this change also cleans up a few issues:

 - 95be717c (parse_opt_ref_sorting: always use with NONEG flag,
   2019-03-20) muses that "git for-each-ref --no-sort" should simply
   clear the sort keys accumulated so far; it now does.

 - The implementation detail of "struct ref_sorting" and the helper
   function parse_ref_sorting() can now be private to the ref-filter
   API implementation.

 - If you set branch.sort to a bogus value, the any "git branch"
   invocation, not only the listing mode, would abort with the
   original code; now it doesn't

Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-10-20 14:33:07 -07:00
Ævar Arnfjörð Bjarmason
e5fb028688 ref-filter API user: add and use a ref_sorting_release()
Add a ref_sorting_release() and use it for some of the current API
users, the ref_sorting_default() function and its siblings will do a
malloc() which wasn't being free'd previously.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-10-20 11:36:13 -07:00
Junio C Hamano
404c4a5462 Merge branch 'ab/designated-initializers'
Code clean-up.

* ab/designated-initializers:
  cbtree.h: define cb_init() in terms of CBTREE_INIT
  *.h: move some *_INIT to designated initializers
  *.h _INIT macros: don't specify fields equal to 0
  *.[ch] *_INIT macros: use { 0 } for a "zero out" idiom
  submodule-config.h: remove unused SUBMODULE_INIT macro
2021-10-11 10:21:48 -07:00
Ævar Arnfjörð Bjarmason
9865b6e6a4 *.[ch] *_INIT macros: use { 0 } for a "zero out" idiom
In C it isn't required to specify that all members of a struct are
zero'd out to 0, NULL or '\0', just providing a "{ 0 }" will
accomplish that.

Let's also change code that provided N zero'd fields to just
provide one, and change e.g. "{ NULL }" to "{ 0 }" for
consistency. I.e. even if the first member is a pointer let's use "0"
instead of "NULL". The point of using "0" consistently is to pick one,
and to not have the reader wonder why we're not using the same pattern
everywhere.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-09-27 14:47:59 -07:00
Jeff King
67985e4e4a refs: drop "broken" flag from for_each_fullref_in()
No callers pass in anything but "0" here. Likewise to our sibling
functions. Note that some of them ferry along the flag, but none of
their callers pass anything but "0" either.

Nor is anybody likely to change that. Callers which really want to see
all of the raw refs use for_each_rawref(). And anybody interested in
iterating a subset of the refs will likely be happy to use the
now-default behavior of showing broken refs, but omitting dangling
symlinks.

So we can get rid of this whole feature.

Signed-off-by: Jeff King <peff@peff.net>
Reviewed-by: Jonathan Tan <jonathantanmy@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-09-27 12:36:45 -07:00
Jeff King
2d653c5036 ref-filter: drop broken-ref code entirely
Now that none of our callers passes the INCLUDE_BROKEN flag, we can drop
it entirely, along with the code to plumb it through to the
for_each_fullref_in() functions.

Signed-off-by: Jeff King <peff@peff.net>
Reviewed-by: Jonathan Tan <jonathantanmy@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-09-27 12:36:45 -07:00
Junio C Hamano
bda891e664 Merge branch 'zh/ref-filter-raw-data'
Prepare the "ref-filter" machinery that drives the "--format"
option of "git for-each-ref" and its friends to be used in "git
cat-file --batch".

* zh/ref-filter-raw-data:
  ref-filter: add %(rest) atom
  ref-filter: use non-const ref_format in *_atom_parser()
  ref-filter: --format=%(raw) support --perl
  ref-filter: add %(raw) atom
  ref-filter: add obj-type check in grab contents
2021-08-24 15:32:37 -07:00
Junio C Hamano
31f9acf9ce Merge branch 'ah/plugleaks'
Leak plugging.

* ah/plugleaks:
  reset: clear_unpack_trees_porcelain to plug leak
  builtin/rebase: fix options.strategy memory lifecycle
  builtin/merge: free found_ref when done
  builtin/mv: free or UNLEAK multiple pointers at end of cmd_mv
  convert: release strbuf to avoid leak
  read-cache: call diff_setup_done to avoid leak
  ref-filter: also free head for ATOM_HEAD to avoid leak
  diffcore-rename: move old_dir/new_dir definition to plug leak
  builtin/for-each-repo: remove unnecessary argv copy to plug leak
  builtin/submodule--helper: release unused strbuf to avoid leak
  environment: move strbuf into block to plug leak
  fmt-merge-msg: free newly allocated temporary strings when done
2021-08-04 13:28:52 -07:00
Andrzej Hunt
d7cf4188e2 ref-filter: also free head for ATOM_HEAD to avoid leak
u.head is populated using resolve_refdup(), which returns a newly
allocated string - hence we also need to free() it.

Found while running t0041 with LSAN:

Direct leak of 16 byte(s) in 1 object(s) allocated from:
    #0 0x486804 in strdup ../projects/compiler-rt/lib/asan/asan_interceptors.cpp:452:3
    #1 0xa8be98 in xstrdup wrapper.c:29:14
    #2 0x9481db in head_atom_parser ref-filter.c:549:17
    #3 0x9408c7 in parse_ref_filter_atom ref-filter.c:703:30
    #4 0x9400e3 in verify_ref_format ref-filter.c:974:8
    #5 0x4f9e8b in print_ref_list builtin/branch.c:439:6
    #6 0x4f9e8b in cmd_branch builtin/branch.c:757:3
    #7 0x4ce83e in run_builtin git.c:475:11
    #8 0x4ccafe in handle_builtin git.c:729:3
    #9 0x4cb01c in run_argv git.c:818:4
    #10 0x4cb01c in cmd_main git.c:949:19
    #11 0x6bdc2d in main common-main.c:52:11
    #12 0x7f96edf86349 in __libc_start_main (/lib64/libc.so.6+0x24349)

SUMMARY: AddressSanitizer: 16 byte(s) leaked in 1 allocation(s).

Signed-off-by: Andrzej Hunt <andrzej@ahunt.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-07-26 12:19:20 -07:00
ZheNing Hu
b9dee075eb ref-filter: add %(rest) atom
%(rest) is a atom used for cat-file batch mode, which can split
the input lines at the first whitespace boundary, all characters
before that whitespace are considered to be the object name;
characters after that first run of whitespace (i.e., the "rest"
of the line) are output in place of the %(rest) atom.

In order to let "cat-file --batch=%(rest)" use the ref-filter
interface, add %(rest) atom for ref-filter.

Introduce the reject_atom() to reject the atom %(rest) for
"git for-each-ref", "git branch", "git tag" and "git verify-tag".

Reviewed-by: Jacob Keller <jacob.keller@gmail.com>
Suggected-by: Jacob Keller <jacob.keller@gmail.com>
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Hariom Verma <hariom18599@gmail.com>
Signed-off-by: ZheNing Hu <adlternative@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-07-26 12:01:26 -07:00
ZheNing Hu
e85fcb355a ref-filter: use non-const ref_format in *_atom_parser()
Use non-const ref_format in *_atom_parser(), which can help us
modify the members of ref_format in *_atom_parser().

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Hariom Verma <hariom18599@gmail.com>
Signed-off-by: ZheNing Hu <adlternative@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-07-26 12:01:26 -07:00
ZheNing Hu
7121c4d4e2 ref-filter: --format=%(raw) support --perl
Because the perl language can handle binary data correctly,
add the function perl_quote_buf_with_len(), which can specify
the length of the data and prevent the data from being truncated
at '\0' to help `--format="%(raw)"` support `--perl`.

Reviewed-by: Jacob Keller <jacob.keller@gmail.com>
Helped-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: ZheNing Hu <adlternative@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-07-26 12:01:25 -07:00
ZheNing Hu
bd0708c7eb ref-filter: add %(raw) atom
Add new formatting option `%(raw)`, which will print the raw
object data without any changes. It will help further to migrate
all cat-file formatting logic from cat-file to ref-filter.

The raw data of blob, tree objects may contain '\0', but most of
the logic in `ref-filter` depends on the output of the atom being
text (specifically, no embedded NULs in it).

E.g. `quote_formatting()` use `strbuf_addstr()` or `*._quote_buf()`
add the data to the buffer. The raw data of a tree object is
`100644 one\0...`, only the `100644 one` will be added to the buffer,
which is incorrect.

Therefore, we need to find a way to record the length of the
atom_value's member `s`. Although strbuf can already record the
string and its length, if we want to replace the type of atom_value's
member `s` with strbuf, many places in ref-filter that are filled
with dynamically allocated mermory in `v->s` are not easy to replace.
At the same time, we need to check if `v->s == NULL` in
populate_value(), and strbuf cannot easily distinguish NULL and empty
strings, but c-style "const char *" can do it. So add a new member in
`struct atom_value`: `s_size`, which can record raw object size, it
can help us add raw object data to the buffer or compare two buffers
which contain raw object data.

Note that `--format=%(raw)` cannot be used with `--python`, `--shell`,
`--tcl`, and `--perl` because if the binary raw data is passed to a
variable in such languages, these may not support arbitrary binary data
in their string variable type.

Reviewed-by: Jacob Keller <jacob.keller@gmail.com>
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Hariom Verma <hariom18599@gmail.com>
Helped-by: Bagas Sanjaya <bagasdotme@gmail.com>
Helped-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Helped-by: Felipe Contreras <felipe.contreras@gmail.com>
Helped-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Helped-by: Junio C Hamano <gitster@pobox.com>
Based-on-patch-by: Olga Telezhnaya <olyatelezhnaya@gmail.com>
Signed-off-by: ZheNing Hu <adlternative@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-07-26 12:01:25 -07:00
ZheNing Hu
311d0b8e8e ref-filter: add obj-type check in grab contents
Only tag and commit objects use `grab_sub_body_contents()` to grab
object contents in the current codebase.  We want to teach the
function to also handle blobs and trees to get their raw data,
without parsing a blob (whose contents looks like a commit or a tag)
incorrectly as a commit or a tag. So it's needed to pass a
`struct expand_data *data` instread of only `void *buf` to both
`grab_sub_body_contents()` and `grab_values()` to be able to check
the object type.

Skip the block of code that is specific to handling commits and tags
early when the given object is of a wrong type to help later
addition to handle other types of objects in this function.

Reviewed-by: Jacob Keller <jacob.keller@gmail.com>
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Hariom Verma <hariom18599@gmail.com>
Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: ZheNing Hu <adlternative@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-07-26 12:01:25 -07:00
Ævar Arnfjörð Bjarmason
48ca53cac4 *.c static functions: add missing __attribute__((format))
Add missing __attribute__((format)) function attributes to various
"static" functions that take printf arguments.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-07-13 15:20:20 -07:00
ZheNing Hu
1197f1a463 ref-filter: introduce enum atom_type
In the original ref-filter design, it will copy the parsed
atom's name and attributes to `used_atom[i].name` in the
atom's parsing step, and use it again for string matching
in the later specific ref attributes filling step. It use
a lot of string matching to determine which atom we need.

Introduce the enum "atom_type", each enum value is named
as `ATOM_*`, which is the index of each corresponding
valid_atom entry. In the first step of the atom parsing,
`used_atom.atom_type` will record corresponding enum value
from valid_atom entry index, and then in specific reference
attribute filling step, only need to compare the value of
the `used_atom[i].atom_type` to check the atom type.

Helped-by: Junio C Hamano <gitster@pobox.com>
Helped-by: Christian Couder <christian.couder@gmail.com>
Signed-off-by: ZheNing Hu <adlternative@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-05-14 06:37:28 +09:00
ZheNing Hu
0caf20f228 ref-filter: add objectsize to used_atom
When the support for "objectsize:disk" was bolted onto the
existing support for "objectsize", it didn't follow the
usual pattern for handling "atomtype:modifier", which reads
the <modifier> part just once while parsing the format
string, and store the parsed result in the union in the
used_atom structure, so that the string form of it does not
have to be parsed over and over at runtime (e.g. in
grab_common_values()).

Add a new member `objectsize` to the union `used_atom.u`,
so that we can separate the check of <modifier> from the
check of <atomtype>, this will bring scalability to atom
`%(objectsize)`.

Signed-off-by: ZheNing Hu <adlternative@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-05-14 06:37:27 +09:00
ZheNing Hu
22f69a85ed ref-filter: get rid of show_ref_array_item
Inlining the exported function `show_ref_array_item()`,
which is not providing the right level of abstraction,
simplifies the API and can unlock improvements at the
former call sites.

Helped-by: René Scharfe <l.s.r@web.de>
Signed-off-by: ZheNing Hu <adlternative@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-04-19 15:08:00 -07:00
Junio C Hamano
f63add4aa8 Merge branch 'jk/ref-filter-segfault-fix'
A NULL-dereference bug has been corrected in an error codepath in
"git for-each-ref", "git branch --list" etc.

* jk/ref-filter-segfault-fix:
  ref-filter: fix NULL check for parse object failure
2021-04-13 15:28:50 -07:00
Jeff King
c685450880 ref-filter: fix NULL check for parse object failure
After we run parse_object_buffer() to get an object's contents, we try
to check that the return value wasn't NULL. However, since our "struct
object" is a pointer-to-pointer, and we assign like:

  *obj = parse_object_buffer(...);

it's not correct to check:

  if (!obj)

That will always be true, since our double pointer will continue to
point to the single pointer (which is itself NULL). This is a regression
that was introduced by aa46a0da30 (ref-filter: use oid_object_info() to
get object, 2018-07-17); since that commit we'll segfault on a parse
failure, as we try to look at the NULL object pointer.

There are many ways a parse could fail, but most of them are hard to set
up in the tests (it's easy to make a bogus object, but update-ref will
refuse to point to it). The test here uses a tag which points to a wrong
object type. A parse of just the broken tag object will succeed, but
seeing both tag objects in the same process will lead to a parse error
(since we'll see the pointed-to object as both types).

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-04-01 12:54:21 -07:00
Junio C Hamano
bfcc6e2a68 Merge branch 'rs/xcalloc-takes-nelem-first'
Code cleanup.

* rs/xcalloc-takes-nelem-first:
  fix xcalloc() argument order
2021-03-19 15:25:39 -07:00
René Scharfe
ca56dadb4b use CALLOC_ARRAY
Add and apply a semantic patch for converting code that open-codes
CALLOC_ARRAY to use it instead.  It shortens the code and infers the
element size automatically.

Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-03-13 16:00:09 -08:00
René Scharfe
241b5d3ebe fix xcalloc() argument order
Pass the number of elements first and ther size second, as expected
by xcalloc().  Provide a semantic patch, which was actually used to
generate the rest of this patch.

The semantic patch would generate flip-flop diffs if both arguments
are sizeofs.  We don't have such a case, and it's hard to imagine
the usefulness of such an allocation.  If it ever occurs then we
could deal with it by duplicating the rule in the semantic patch to
make it cancel itself out, or we could change the code to use
CALLOC_ARRAY.

Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-03-08 09:45:04 -08:00
Junio C Hamano
28714238c8 Merge branch 'hv/trailer-formatting'
The logic to handle "trailer" related placeholders in the
"--format=" mechanisms in the "log" family and "for-each-ref"
family is getting unified.

* hv/trailer-formatting:
  ref-filter: use pretty.c logic for trailers
  pretty.c: capture invalid trailer argument
  pretty.c: refactor trailer logic to `format_set_trailers_options()`
  t6300: use function to test trailer options
2021-03-01 14:02:58 -08:00
Junio C Hamano
15af6e6fee Merge branch 'bc/signed-objects-with-both-hashes'
Signed commits and tags now allow verification of objects, whose
two object names (one in SHA-1, the other in SHA-256) are both
signed.

* bc/signed-objects-with-both-hashes:
  gpg-interface: remove other signature headers before verifying
  ref-filter: hoist signature parsing
  commit: allow parsing arbitrary buffers with headers
  gpg-interface: improve interface for parsing tags
  commit: ignore additional signatures when parsing signed commits
  ref-filter: switch some uses of unsigned long to size_t
2021-02-22 16:12:42 -08:00
Hariom Verma
ee82a487f6 ref-filter: use pretty.c logic for trailers
Now, ref-filter is using pretty.c logic for setting trailer options.

New to ref-filter:
  :key=<K> - only show trailers with specified key.
  :valueonly[=val] - only show the value part.
  :separator=<SEP> - inserted between trailer lines.
  :key_value_separator=<SEP> - inserted between key and value in trailer lines

Enhancement to existing options(now can take value and its optional):
  :only[=val]
  :unfold[=val]

'val' can be: true, on, yes or false, off, no.

Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Mentored-by: Heba Waly <heba.waly@gmail.com>
Signed-off-by: Hariom Verma <hariom18599@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-02-15 16:48:38 -08:00
brian m. carlson
88bce0e24c ref-filter: hoist signature parsing
When we parse a signature in the ref-filter code, we continually
increment the buffer pointer.  Hoist the signature parsing above the
blank line delimiting headers and body so we can find the signature when
using a header to sign the buffer.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-02-10 23:35:42 -08:00
brian m. carlson
482c119186 gpg-interface: improve interface for parsing tags
We have a function which parses a buffer with a signature at the end,
parse_signature, and this function is used for signed tags.  However,
we'll need to store values for multiple algorithms, and we'll do this by
using a header for the non-default algorithm.

Adjust the parse_signature interface to store the parsed data in two
strbufs and turn the existing function into parse_signed_buffer.  The
latter is still used in places where we know we always have a signed
buffer, such as push certs.

Adjust all the callers to deal with this new interface.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-02-10 23:35:42 -08:00
Junio C Hamano
6254fa1359 Merge branch 'tb/ls-refs-optim'
The ls-refs protocol operation has been optimized to narrow the
sub-hierarchy of refs/ it walks to produce response.

* tb/ls-refs-optim:
  ls-refs.c: traverse prefixes of disjoint "ref-prefix" sets
  ls-refs.c: initialize 'prefixes' before using it
  refs: expose 'for_each_fullref_in_prefixes'
2021-02-05 16:40:45 -08:00
Taylor Blau
16b1985be5 refs: expose 'for_each_fullref_in_prefixes'
This function was used in the ref-filter.c code to find the longest
common prefix of among a set of refspecs, and then to iterate all of the
references that descend from that prefix.

A future patch will want to use that same code from ls-refs.c, so
prepare by exposing and moving it to refs.c. Since there is nothing
specific to the ref-filter code here (other than that it was previously
the only caller of this function), this really belongs in the more
generic refs.h header.

The code moved in this patch is identical before and after, with the one
exception of renaming some arguments to be consistent with other
functions exposed in refs.h.

Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-01-22 18:57:27 -08:00
brian m. carlson
83dff3eb2e ref-filter: switch some uses of unsigned long to size_t
In the future, we'll want to pass some of the arguments of find_subpos
to strbuf_detach, which takes a size_t.  This is fine on systems where
that's the same size as unsigned long, but that isn't the case on all
systems.  Moreover, size_t makes sense since it's not possible to use a
buffer here that's larger than memory anyway.

Let's switch each use to size_t for these lengths in
grab_sub_body_contents and find_subpos.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-01-18 17:38:19 -08:00
Ævar Arnfjörð Bjarmason
4045f659bd branch: show "HEAD detached" first under reverse sort
Change the output of the likes of "git branch -l --sort=-objectsize"
to show the "(HEAD detached at <hash>)" message at the start of the
output. Before the compare_detached_head() function added in a
preceding commit we'd emit this output as an emergent effect.

It doesn't make any sense to consider the objectsize, type or other
non-attribute of the "(HEAD detached at <hash>)" message for the
purposes of sorting. Let's always emit it at the top instead. The only
reason it was sorted in the first place is because we're injecting it
into the ref-filter machinery so builtin/branch.c doesn't need to do
its own "am I detached?" detection.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-01-07 15:13:21 -08:00
Ævar Arnfjörð Bjarmason
2708ce62d2 branch: sort detached HEAD based on a flag
Change the ref-filter sorting of detached HEAD to check the
FILTER_REFS_DETACHED_HEAD flag, instead of relying on the ref
description filled-in by get_head_description() to start with "(",
which in turn we expect to ASCII-sort before any other reference.

For context, we'd like the detached line to appear first at the start
of "git branch -l", e.g.:

    $ git branch -l
    * (HEAD detached at <hash>)
      master

This doesn't change that, but improves on a fix made in
28438e84e0 (ref-filter: sort detached HEAD lines firstly, 2019-06-18)
and gives the Chinese translation the ability to use its preferred
punctuation marks again.

In Chinese the fullwidth versions of punctuation like "()" are
typically written as (U+FF08 fullwidth left parenthesis), (U+FF09
fullwidth right parenthesis) instead[1]. This form is used in both
po/zh_{CN,TW}.po in most cases where "()" is translated in a string.

Aside from that improvement to the Chinese translation, it also just
makes for cleaner code that we mark any special cases in the ref_array
we're sorting with flags and make the sort function aware of them,
instead of piggy-backing on the general-case of strcmp() doing the
right thing.

As seen in the amended tests this made reverse sorting a bit more
consistent. Before this we'd sometimes sort this message in the
middle, now it's consistently at the beginning or end, depending on
whether we're doing a normal or reverse sort. Having it at the end
doesn't make much sense either, but at least it behaves consistently
now. A follow-up commit will make this behavior under reverse sorting
even better.

I'm removing the "TRANSLATORS" comments that were in the old code
while I'm at it. Those were added in d4919bb288 (ref-filter: move
get_head_description() from branch.c, 2017-01-10). I think it's
obvious from context, string and translation memory in typical
translation tools that these are the same or similar string.

1. https://en.wikipedia.org/wiki/Chinese_punctuation#Marks_similar_to_European_punctuation

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-01-07 15:13:21 -08:00
Ævar Arnfjörð Bjarmason
7c269a7b16 ref-filter: move ref_sorting flags to a bitfield
Change the reverse/ignore_case/version sort flags in the ref_sorting
struct into a bitfield. Having three of them was already a bit
unwieldy, but it would be even more so if another flag needed a
function like ref_sorting_icase_all() introduced in
76f9e569ad (ref-filter: apply --ignore-case to all sorting keys,
2020-05-03).

A follow-up change will introduce such a flag, so let's move this over
to a bitfield. Instead of using the usual '#define' pattern I'm using
the "enum" pattern from builtin/rebase.c's b4c8eb024a (builtin
rebase: support --quiet, 2018-09-04).

Perhaps there's a more idiomatic way of doing the "for each in list
amend mask" pattern than this "mask/on" variable combo. This function
doesn't allow us to e.g. do any arbitrary changes to the bitfield for
multiple flags, but I think in this case that's fine. The common case
is that we're calling this with a list of one.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-01-07 15:13:21 -08:00