Re: [PATCH 80/83] run-command: make dup_devnull() non static

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

 



Duy Nguyen <pclouds@xxxxxxxxx> writes:

> I vote one step at a time, leave multi-thread support for future.
> There's a lot more shared state than file descriptors anyway, at least
> there are object db and index access and probably a couple of hidden
> static variables somewhere. And I'm not sure if multi-thread really
> helps here. Are we really CPU-bound? If object inflation causes that
> (wild guess), can we just inflate ahead in some separate process and
> pass the result back?

I do not particularly care about multi-thread issues, but I have to
agree with Dscho that the updated code that claims to be "libified"
that futz with the file descriptors like the way this patch does is
not a proper libification.

Unfortunately, the anticipated caller of this code that does "this
may fail and it is OK because it is merely one of the attempts, so
let's not show the errors" is not something we call only when we are
falling back, so "why not do this rare codepath via the usual
run_command() interface to spawn 'apply' as a separate process?",
which would be the most sensible "one step at a time" suggestion if
it were the case, would not apply.

As you will be passing the apply state structure throughout the
callchain, would it be a viable and reasonable endgame state to have
a strbuf in it that accumulates the errors?  That is, instead of
dup()ing the standard error stream out, you would accumulate the
errors for a caller that asks their errors not directly sent to
the standard error stream, so that it can choose to either show it
at the end, or ignore it altogether.

How far can you go with just set-error-routine?  Are there things,
other than the file descriptors, that you need to futz with in order
to covert that "we'd fallback, so this early round must be silent"
codepath?
--
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]