Re: [PATCH/RFC 2/2] change all unchecked calls to setenv to xsetenv

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

 



On Tue, Dec 13, 2011 at 7:33 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> Jeff King <peff@xxxxxxxx> writes:
>
>> On Tue, Dec 13, 2011 at 01:10:27PM +0100, Erik Faye-Lund wrote:
>>
>>> While reviewing some patches for Git for Windows, I realized that
>>> we almost never check the return-value from setenv. This can lead
>>> to quite surprising errors in unusual sitations. Mostly, an error
>>> would probably be preferred. So here we go.
>>>
>>> However, I'm not at all convinced myself that all of these make
>>> sense; in particular settings like GIT_EDITOR and GIT_PAGER could
>>> perhaps benefit from having a warning printed rather than a hard
>>> error.
>>>
>>> Thoughts?
>>
>> I wrote almost the same patch once[1], but failed to actually push it
>> through to acceptance.
>
> In both cases, the patches are _designed_ to fail to attract any
> attention.  Your earlier one had this preamble:
>
>  Here is a patch. I still feel a little silly writing this. The chances
>  that you will run out of memory doing setenv but _not_ doing any of the
>  other git operations seems very low.
>
> which rather *loudly* says "ignore me, please!" ;-)
>
> Erik's patch this round is no better; if its log message said something
> like "On platform X, the environment space is merely nKB and setenv has
> much higher chance of failing than on typical Linux boxes", it would have
> been a no brainer for me to respond with "makes perfect sense but don't we
> also use putenv?" immediately.
>

It could be because I treated this completely like a theoretical
patch; I haven't seen it actually happen.

But you are right, Windows 32 kB environment limit makes this much
more likely than your average Linux box. So perhaps I should add a
notice about that in the next round...
--
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]