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? -Emil _______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
