Commit Graph

20 Commits

Author SHA1 Message Date
Eric Wong
404ab78e39 hashmap: remove type arg from hashmap_{get,put,remove}_entry
Since these macros already take a `keyvar' pointer of a known type,
we can rely on OFFSETOF_VAR to get the correct offset without
relying on non-portable `__typeof__' and `offsetof'.

Argument order is also rearranged, so `keyvar' and `member' are
sequential as they are used as: `keyvar->member'

Signed-off-by: Eric Wong <e@80x24.org>
Reviewed-by: Derrick Stolee <stolee@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-10-07 10:20:12 +09:00
Eric Wong
939af16eac hashmap_cmp_fn takes hashmap_entry params
Another step in eliminating the requirement of hashmap_entry
being the first member of a struct.

Signed-off-by: Eric Wong <e@80x24.org>
Reviewed-by: Derrick Stolee <stolee@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-10-07 10:20:11 +09:00
Eric Wong
f23a465132 hashmap_get{,_from_hash} return "struct hashmap_entry *"
Update callers to use hashmap_get_entry, hashmap_get_entry_from_hash
or container_of as appropriate.

This is another step towards eliminating the requirement of
hashmap_entry being the first field in a struct.

Signed-off-by: Eric Wong <e@80x24.org>
Reviewed-by: Derrick Stolee <stolee@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-10-07 10:20:11 +09:00
Eric Wong
28ee794128 hashmap_remove takes "const struct hashmap_entry *"
This is less error-prone than "const void *" as the compiler
now detects invalid types being passed.

Signed-off-by: Eric Wong <e@80x24.org>
Reviewed-by: Derrick Stolee <stolee@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-10-07 10:20:10 +09:00
Eric Wong
b6c5241606 hashmap_get takes "const struct hashmap_entry *"
This is less error-prone than "const void *" as the compiler
now detects invalid types being passed.

Signed-off-by: Eric Wong <e@80x24.org>
Reviewed-by: Derrick Stolee <stolee@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-10-07 10:20:10 +09:00
Eric Wong
b94e5c1df6 hashmap_add takes "struct hashmap_entry *"
This is less error-prone than "void *" as the compiler now
detects invalid types being passed.

Signed-off-by: Eric Wong <e@80x24.org>
Reviewed-by: Derrick Stolee <stolee@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-10-07 10:20:10 +09:00
Eric Wong
d22245a2e3 hashmap_entry_init takes "struct hashmap_entry *"
C compilers do type checking to make life easier for us.  So
rely on that and update all hashmap_entry_init callers to take
"struct hashmap_entry *" to avoid future bugs while improving
safety and readability.

Signed-off-by: Eric Wong <e@80x24.org>
Reviewed-by: Derrick Stolee <stolee@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-10-07 10:20:09 +09:00
Jeff Hostetler
0671b4d193 trace2:data: add trace2 sub-process classification
Add trace2 classification for long-running processes
started in sub-process.c

Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-02-22 15:28:12 -08:00
Junio C Hamano
98c03a0de8 Merge branch 'tg/memfixes'
Fixes for a handful memory access issues identified by valgrind.

* tg/memfixes:
  sub-process: use child_process.args instead of child_process.argv
  http-push: fix construction of hex value from path
  path.c: fix uninitialized memory access
2017-10-07 16:27:54 +09:00
Johannes Sixt
2944a94c6b sub-process: use child_process.args instead of child_process.argv
Currently the argv is only allocated on the stack, and then assigned to
process->argv.  When the start_subprocess function goes out of scope,
the local argv variable is eliminated from the stack, but the pointer is
still kept around in process->argv.

Much later when we try to access the same process->argv in
finish_command, this leads us to access a memory location that no longer
contains what we want.  As argv0 is only used for printing errors, this
is not easily noticed in normal git operations.  However when running
t0021-conversion.sh through valgrind, valgrind rightfully complains:

==21024== Invalid read of size 8
==21024==    at 0x2ACF64: finish_command (run-command.c:869)
==21024==    by 0x2D6B18: subprocess_exit_handler (sub-process.c:72)
==21024==    by 0x2AB41E: cleanup_children (run-command.c:45)
==21024==    by 0x2AB526: cleanup_children_on_exit (run-command.c:81)
==21024==    by 0x54AD487: __run_exit_handlers (in /usr/lib/libc-2.26.so)
==21024==    by 0x54AD4D9: exit (in /usr/lib/libc-2.26.so)
==21024==    by 0x11A9EF: handle_builtin (git.c:550)
==21024==    by 0x11ABCC: run_argv (git.c:602)
==21024==    by 0x11AD8E: cmd_main (git.c:679)
==21024==    by 0x1BF125: main (common-main.c:43)
==21024==  Address 0x1ffeffec00 is on thread 1's stack
==21024==  1504 bytes below stack pointer
==21024==

These days, the child_process structure has its own args array, and
the standard way to set up its argv[] is to use that one, instead of
assigning to process->argv to point at an array that is outside.
Use that facility automatically fixes this issue.

Reported-by: Thomas Gummerer <t.gummerer@gmail.com>
Signed-off-by: Johannes Sixt <j6t@kdbg.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-10-04 13:58:15 +09:00
Junio C Hamano
b9db14f52e Merge branch 'cc/subprocess-handshake-missing-capabilities'
Finishing touches to a topic already in 'master'.

* cc/subprocess-handshake-missing-capabilities:
  subprocess: loudly die when subprocess asks for an unsupported capability
2017-09-25 15:24:06 +09:00
Junio C Hamano
ab46e6fc72 subprocess: loudly die when subprocess asks for an unsupported capability
The handshake_capabilities() function first advertises the set of
capabilities it supports, so that the other side can pick and choose
which ones to use and ask us to enable in its response.  Then we
read the response that tells us what choice the other side made.  If
we saw something that we never advertised, that indicates one of two
things.  The other side, i.e. the "upgraded" filter, is not paying
attention of the capabilities advertisement, and asking something
its correct operation relies on, but we are not capable of giving
that unknown feature and operate without it, so after that point the
exchange of data is a garbage-in-garbage-out.  Or the other side
wanted to ask for one of the capabilities we advertised, but the
code has typo and their wish to enable a capability that its correct
operation relies on is not understood on this end.  The result is
the same garbage-in-garbage-out.

Instead of sweeping such a potential bug under the rug, die loudly
when we see a request for an unsupported capability in order to
force sloppily-written filter scripts to get corrected.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-09-11 12:21:29 +09:00
Junio C Hamano
d1615f93ac Merge branch 'cc/subprocess-handshake-missing-capabilities'
When handshake with a subprocess filter notices that the process
asked for an unknown capability, Git did not report what program
the offending subprocess was running.  This has been corrected.

* cc/subprocess-handshake-missing-capabilities:
  sub-process: print the cmd when a capability is unsupported
2017-08-24 10:20:02 -07:00
Christian Couder
d3ba566342 sub-process: print the cmd when a capability is unsupported
In handshake_capabilities() we use warning() when a capability
is not supported, so the exit code of the function is 0 and no
further error is shown. This is a problem because the warning
message doesn't tell us which subprocess cmd failed.

On the contrary if we cannot write a packet from this function,
we use error() and then subprocess_start() outputs:

    initialization for subprocess '<cmd>' failed

so we can know which subprocess cmd failed.

Let's improve the warning() message, so that we can know which
subprocess cmd failed.

Helped-by: Lars Schneider <larsxschneider@gmail.com>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-08-16 09:40:33 -07:00
Junio C Hamano
9a8ff899ce Merge branch 'jt/subprocess-handshake'
Code cleanup.

* jt/subprocess-handshake:
  sub-process: refactor handshake to common function
  Documentation: migrate sub-process docs to header
2017-08-11 13:27:05 -07:00
Jonathan Tan
fa64a2fdbe sub-process: refactor handshake to common function
Refactor, into a common function, the version and capability negotiation
done when invoking a long-running process as a clean or smudge filter.
This will be useful for other Git code that needs to interact similarly
with a long-running process.

As you can see in the change to t0021, this commit changes the error
message reported when the long-running process does not introduce itself
with the expected "server"-terminated line. Originally, the error
message reports that the filter "does not support filter protocol
version 2", differentiating between the old single-file filter protocol
and the new multi-file filter protocol - I have updated it to something
more generic and useful.

Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-07-26 13:00:40 -07:00
Stefan Beller
9ab42958f6 convert/sub-process: drop cast to hashmap_cmp_fn
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-07-05 13:53:12 -07:00
Stefan Beller
7663cdc86c hashmap.h: compare function has access to a data field
When using the hashmap a common need is to have access to caller provided
data in the compare function. A couple of times we abuse the keydata field
to pass in the data needed. This happens for example in patch-ids.c.

This patch changes the function signature of the compare function
to have one more void pointer available. The pointer given for each
invocation of the compare function must be defined in the init function
of the hashmap and is just passed through.

Documentation of this new feature is deferred to a later patch.
This is a rather mechanical conversion, just adding the new pass-through
parameter.  However while at it improve the naming of the fields of all
compare functions used by hashmaps by ensuring unused parameters are
prefixed with 'unused_' and naming the parameters what they are (instead
of 'unused' make it 'unused_keydata').

Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-06-30 12:49:28 -07:00
Ben Peart
4f2a2e9f0e convert: update subprocess_read_status() to not die on EOF
Enable sub-processes to gracefully handle when the process dies by
updating subprocess_read_status to return an error on EOF instead of
dying.

Update apply_multi_file_filter to take advantage of the revised
subprocess_read_status.

Signed-off-by: Ben Peart <benpeart@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-05-15 13:01:57 +09:00
Ben Peart
99605d62e8 sub-process: move sub-process functions into separate files
Move the sub-proces functions into sub-process.h/c.  Add documentation
for the new module in Documentation/technical/api-sub-process.txt

Signed-off-by: Ben Peart <benpeart@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-05-15 13:01:57 +09:00