This is the mail archive of the cygwin mailing list for the Cygwin project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Excessive thrashing when popen()ing after a large malloc()


Eric Blake <ebb9 <at> byu.net> writes:
> Loic Grenie <grenie <at> matapp.unimib.it> writes:
>
>>     That intermediate sh invocation can probably be slightly streamlined
>>   by doing something similar to
>> 
>> sprintf(cmd, "(%s) >&%d %d>&- %d>&-", program, pdes[1], pdes[0], pdes[1]);
>> pid = spawnl(_P_NOWAIT, _PATH_BSHELL, "sh", "-c", cmd, NULL);
>
> Nope.
> Still one fork too many due to the use of a subshell

    Some shells do not for if the subshell is the last command of the shell
  (don't know about bash though). And it's much better to fork a 500kb
  bash twice than a 1GB program once (in case the computer is so memory
  constrained that forking bash once more is a large performance penalty,
  popen() is probably a bad idea in the first place). The more I think about
  it, the more I wish Windows were UNIX. 

> still mishandles any ' in program,

    Probably true but I don't see why. I don't use ' anymore and it looks to
  me that the subshell should insulate the '.

> and regresses in robustness since it would mishandle a 
> program such as "echo hi);(:" that is currently rejected by popen.

    Correct.
    What about this really really backslashed idea:

cmd = malloc(2*strlen(program)+64);
sprintf(cmd, "eval \"");
for (p = cmd+strlen(cmd), q = program; *q;) {
    /* if (!isalphanum(*q)) */
    *p++ = '\\';
    *p++ = *q++;
}
sprintf(p, "\" >&%d %d>&- %d>&-", pdes[1], pdes[0], pdes[1]);
pid = spawnl(_P_NOWAIT, _PATH_BSHELL, "sh", "-c", cmd, NULL);

  No more forking, I hope there is no mishandling of metacharacters and it
  fails for syntactically incorrect program.

>> 
>>   but you misunderstand my objective: I'm not "trying to have my patch
>>   accepted", I'm just signaling a problem and proposing a patch for the
>>   upstream maintainers: cygwin and/or newlib. I've modified the program
>>   that uses popen and it is now using a _popen which is a kludge around
>>   the ideas that I have suggested. If my ideas are useful for anybody else,
>>   I'm really happy to contribute and help; if nobody cares, neither do I.

> Indeed, ANY time you don't like what the standard library provides, you are 
> ALWAYS welcome to write your own replacement that suits your needs (and that 
> goes for more than just popen).  And the fact that you reported to this list
> is a good thing, as it looks like you spurred some developmental ideas.

> In other words, we do care about performance, but we also care about
> maintainability, standards documents, and backwards compatibility; it's not
> easy to balance all of these (sometimes competing) objectives.  I'll just
> wait to see the patch that cgf has in his sandbox.

    Indeed. Since I've no experience about maintainability, no idea how to
  test that the patch does not break anything and no POSIX standard document
  (and neither the time to read, analyze and understand it), I've submitted my
  dirty two-liner to the list. And I try to improve it anytime some smarter
  or more knowledgeable guy suggests there is a problem.

> -- 
> Eric Blake

     Loïc

--
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple
Problem reports:       http://cygwin.com/problems.html
Documentation:         http://cygwin.com/docs.html
FAQ:                   http://cygwin.com/faq/


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]