checked sources of xorg-server 1.7.0 with static code analysis tool cppcheck

[Ping]

>
> On Oct 3, 2009, at 03:53, Michel Dänzer wrote:
>
> On Fri, 2009-10-02 at 18:25 -0700, Jeremy Huddleston wrote:
>>
>>> On Oct 2, 2009, at 16:17, Michel Dänzer wrote:
>>>
>>> On Fri, 2009-10-02 at 23:10 +0200, Martin Ettl wrote:
>>>>
>>>>>
>>>>> diff --git a/exa/exa_classic.c b/exa/exa_classic.c
>>>>> index 1eff570..c9c7534 100644
>>>>> --- a/exa/exa_classic.c
>>>>> +++ b/exa/exa_classic.c
>>>>> @@ -144,14 +144,14 @@ Bool
>>>>> exaModifyPixmapHeader_classic(PixmapPtr pPixmap, int width, int
>>>>> height, int depth,
>>>>>                    int bitsPerPixel, int devKind, pointer pPixData)
>>>>> {
>>>>> +    if (!pPixmap)
>>>>> +        return FALSE;
>>>>> +
>>>>>   ScreenPtr pScreen = pPixmap->drawable.pScreen;
>>>>>   ExaScreenPrivPtr pExaScr;
>>>>>   ExaPixmapPrivPtr pExaPixmap;
>>>>>   Bool ret;
>>>>>
>>>>> -    if (!pPixmap)
>>>>> -        return FALSE;
>>>>> -
>>>>>   pExaScr = ExaGetScreenPriv(pScreen);
>>>>>   pExaPixmap = ExaGetPixmapPriv(pPixmap);
>>>>>
>>>>> ...
>>>> Mixing code and declarations is a C99 feature, and I'm not sure we
>>>> require a C99 compiler yet. I'm not sure that that these tests serve
>>>> any
>>>> real purpose, they can probably just be removed.
>>>>
>>>
>>> Well, they serve a purpose, and I'd like to keep them in for sanity
>>> purposes.
>>>
>>
>> What purpose is that? If these functions were actually called with a
>> NULL PixmapPtr, surely the current code would have crashed with a
>> segmentation fault.
>
>
My experience is "don't change it unless you know it is broken".  Gracefully
returning a warning (FALSE) is better than making yourself (xserver) look
bad (crashing).

Ping
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.x.org/archives/xorg-devel/attachments/20091005/92d12b55/attachment-0001.htm