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

[Jeremy Huddleston]

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.


Right, but the way I look at it:

1) That code was added to git about 6 weeks ago
2) You don't know how that code might be called in the future.  Better  
safe than sorry.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/pkcs7-signature
Size: 3333 bytes
Desc: not available
Url : http://lists.x.org/archives/xorg-devel/attachments/20091005/e725cdb1/attachment-0001.bin