[Xcb] [PATCH] check if allocated image is NULL _before_ first derefence

[Henning Sten]

The way I was thinking about it was that it's a good idea to make sure that null checks always happen immediately after the allocation. If there is space between the allocation and the null check there is an increased risk that other people add stuff between the allocation and the null check and eventually it might not be obvious that the variable has not yet been null checked. For example:

x = malloc(...)
if (!x) {
....
}

Also, I've always considered "mixing declaration and code" to be a feature not a bad habit. Of course, in some code bases it's necessary to support old compilers but the xcb-util code seems to be mixing declaration and code already, right? Or is it per block and not per function?

If declarations are placed close to where the variable is used, then it becomes easier to read the code (easier to see what type stuff has). And GCC will more often be able to find unused variables when they are mixed in with the code (as explained here: http://blog.flameeyes.eu/articles/2008/05/11/good-reasons-to-mix-declarations-and-code-unused-variables ).

I'd be very interested to hear why you think declarations/code should not be mixed. Is it because we need to support some particular old compiler or maybe some particular C standard?

Anyway; I'm attaching a second version which I think addresses your concern while still making sure allocation/null-check is next to each other.



HS

--- On Fri, 9/26/08, Julien Cristau <jcristau at debian.org> wrote:
> From: Julien Cristau <jcristau at debian.org>
> Subject: Re: [Xcb] [PATCH] check if allocated image is NULL _before_ first derefence
> To: "Henning Sten" <henning.sten at yahoo.com>
> Cc: xcb at lists.freedesktop.org
> Date: Friday, September 26, 2008, 10:22 PM
> Hi,
> 
> Your patch mixes code and declarations.  I'd move the
> dst_plane
> initialization below the NULL check instead.
> 
> Cheers,
> Julien


      
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-avoid-dereference-before-null-check-while-also-keepi.patch
Type: text/x-patch
Size: 1585 bytes
Desc: not available
Url : http://lists.freedesktop.org/archives/xcb/attachments/20080926/fbbabaac/attachment.bin