On 03/ 7/18 05:36 AM, Emil Velikov wrote:
> Hi Alan,
>
> On 6 March 2018 at 21:47, Alan Coopersmith <[email protected]>
> wrote:
>> Reported by gcc 7.3:
>>
>> GetImage.c:110:25: warning: potential null pointer dereference
>> [-Wnull-dereference]
>> if (planes < 1 || image->height < 1 || image->bytes_per_line < 1 ||
>> ~~~~~^~~~~~~~
>>
>> Introduced by 8ea762f94f4c942d898fdeb590a1630c83235c17 in Xlib 1.6.4
>>
>> Signed-off-by: Alan Coopersmith <[email protected]>
>> ---
>> src/GetImage.c | 16 +++++++++-------
>> 1 file changed, 9 insertions(+), 7 deletions(-)
>>
>> diff --git a/src/GetImage.c b/src/GetImage.c
>> index ff32d589..44a576a1 100644
>> --- a/src/GetImage.c
>> +++ b/src/GetImage.c
>> @@ -105,14 +105,16 @@ XImage *XGetImage (
>> planes = 1;
>> }
>>
>> - if (!image)
>> + if (!image) {
>> Xfree(data);
>> - if (planes < 1 || image->height < 1 || image->bytes_per_line < 1 ||
>> - INT_MAX / image->height <= image->bytes_per_line ||
>> - INT_MAX / planes <= image->height * image->bytes_per_line ||
>> - nbytes < planes * image->height * image->bytes_per_line) {
>> - XDestroyImage(image);
>> - image = NULL;
>> + } else {
>> + if (planes < 1 || image->height < 1 || image->bytes_per_line <
>> 1 ||
>> + INT_MAX / image->height <= image->bytes_per_line ||
>> + INT_MAX / planes <= image->height * image->bytes_per_line ||
>> + nbytes < planes * image->height * image->bytes_per_line) {
>> + XDestroyImage(image);
>> + image = NULL;
>> + }
>
> Instead of reshuffling, one could easily add the missing unlock/sync
> in the error path.
> Or even a goto unlock?
Sorry - not sure I'm following - are you suggesting something like this?
--- a/src/GetImage.c
+++ b/src/GetImage.c
@@ -104,17 +104,20 @@ XImage *XGetImage (
_XGetScanlinePad(dpy, (int) rep.depth), 0);
planes = 1;
}
- if (!image)
+ if (!image) {
Xfree(data);
+ goto unlock;
+ }
if (planes < 1 || image->height < 1 || image->bytes_per_line < 1 ||
INT_MAX / image->height <= image->bytes_per_line ||
INT_MAX / planes <= image->height * image->bytes_per_line ||
nbytes < planes * image->height * image->bytes_per_line) {
XDestroyImage(image);
image = NULL;
}
+ unlock:
UnlockDisplay(dpy);
SyncHandle();
return (image);
}
That should be effectively equivalent, just less change in indentation for
the lines in between.
--
-Alan Coopersmith- [email protected]
Oracle Solaris Engineering - https://blogs.oracle.com/alanc
_______________________________________________
[email protected]: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel
