Re: [PATCH 1/2] Move run_hook() from builtin-commit.c into run-command.c (libgit)

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

> > -	ret = start_command(&hook);
> > -	if (ret) {
> > -		warning("Could not spawn %s", argv[0]);
> > -		return ret;
> > -	}
> > -	ret = finish_command(&hook);
> > -	if (ret == -ERR_RUN_COMMAND_WAITPID_SIGNAL)
> > -		warning("%s exited due to uncaught signal", argv[0]);
> 
> What are the side effects of replacing this with "ret = 
> run_command(&hook);"?  This has to be discussed and defended in the commit 
> message.

This is a very good point.
The consequences are that two warnings are missing, but these are
warnings that are useful enough to be included for all those hooks,
imho.

Thanks!

> Lots of improvements possible (I agree; _after_ this patch):
[...]
> - use ALLOC_GROW instead of having a fixed size argv,

Agreed.

> - use an strbuf for the index file

Is that useful in some way?
Currently it would only adds code to generate strbufs at the caller
side. And in the case the caller has a strbuf for the index file, it
can simply use the .buf member.

> - checking executability of argv[0] before filling argv,

Agreed.

Patch series v2 will follow.

Thanks,
  Stephan

-- 
Stephan Beyer <s-beyer@xxxxxxx>, PGP 0x6EDDD207FCC5040F
--
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

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux