git/sub-process.c, branch v2.18.2

Merge branch 'tg/memfixes'

2017-10-07T07:27:54Z

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

sub-process: use child_process.args instead of child_process.argv

2017-10-04T04:58:15Z

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 Signed-off-by: Johannes Sixt Signed-off-by: Junio C Hamano

Merge branch 'cc/subprocess-handshake-missing-capabilities'

2017-09-25T06:24:06Z

Finishing touches to a topic already in 'master'. * cc/subprocess-handshake-missing-capabilities: subprocess: loudly die when subprocess asks for an unsupported capability

subprocess: loudly die when subprocess asks for an unsupported capability

2017-09-11T03:21:29Z

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

Merge branch 'cc/subprocess-handshake-missing-capabilities'

2017-08-24T17:20:02Z

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

sub-process: print the cmd when a capability is unsupported

2017-08-16T16:40:33Z

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 '' 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 Signed-off-by: Christian Couder Signed-off-by: Junio C Hamano

Merge branch 'jt/subprocess-handshake'

2017-08-11T20:27:05Z

Code cleanup. * jt/subprocess-handshake: sub-process: refactor handshake to common function Documentation: migrate sub-process docs to header

sub-process: refactor handshake to common function

2017-07-26T20:00:40Z

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 Signed-off-by: Junio C Hamano

convert/sub-process: drop cast to hashmap_cmp_fn

2017-07-05T20:53:12Z

Signed-off-by: Stefan Beller Signed-off-by: Junio C Hamano

hashmap.h: compare function has access to a data field

2017-06-30T19:49:28Z

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 Signed-off-by: Junio C Hamano