From: Erik Faye-Lund <kusmabite@gmail.com>
Date: Tue, 23 Nov 2010 18:00:28 GMT
Subject: Re: [RFC PATCH 1/6] msvc: opendir: use xmalloc
Message-ID: <AANLkTin01j-yb1j7WsCkZ2MThJ80qbcOw5yjE5LxrqJZ@mail.gmail.com>
In-Reply-To: <AANLkTiknTQp96PeiBd+MqCO6uEs1mCzFYDsW8NwYpBsm@mail.gmail.com>

On Tue, Nov 23, 2010 at 6:45 PM, Erik Faye-Lund <kusmabite@gmail.com> wrote:
> On Tue, Nov 23, 2010 at 6:40 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:
>> Erik Faye-Lund wrote:
>>
>>> --- a/compat/msvc.c
>>> +++ b/compat/msvc.c
>>> @@ -7,16 +7,13 @@ DIR *opendir(const char *name)
>>>  {
>>>       int len;
>>>       DIR *p;
>>> -     p = (DIR*)malloc(sizeof(DIR));
>>> +     p = xmalloc(sizeof(DIR));
>>>       memset(p, 0, sizeof(DIR));
>>>       strncpy(p->dd_name, name, PATH_MAX);
>>>       len = strlen(p->dd_name);
>>>       p->dd_name[len] = '/';
>>>       p->dd_name[len+1] = '*';
>>>
>>> -     if (p == NULL)
>>> -             return NULL;
>>
>> A behavior change but maybe a good one.  For example, the
>> prune_packed_objects() loop currently skips object dirs it can't open,
>> even if that is due to memory exhaustion, but this changes it to error
>> out.
>>
>> What is the motivation?
>>
>
> The motivation is just to avoid having to deal with the error, like we
> do other places. It's not a big deal though. I could also set errno to
> ENOMEM and return NULL if that's preferable. I just don't see how it
> is.
>
> I also slightly dislike setting an error not listed in POSIX'
> documentation of opendir, even though it's probably allowed.
>

Of course, I don't have to set errno; malloc would already have done
that. All I need to do is move the check for NULL a bit earlier, so it
won't segfault on ENOMEM. I'll change it for the next round.