Discussion:
[Mesa-dev] [PATCH] egl/x11: Remove unneeded free() on always null string
v***@gmail.com
2017-12-01 15:08:53 UTC
Permalink
From: Vadym Shovkoplias <***@globallogic.com>

In this condition dri2_dpy->driver_name string always equals
NULL, so call to free() is useless

Signed-off-by: Vadym Shovkoplias <***@globallogic.com>

---
src/egl/drivers/dri2/platform_x11.c | 1 -
1 file changed, 1 deletion(-)

diff --git a/src/egl/drivers/dri2/platform_x11.c b/src/egl/drivers/dri2/platform_x11.c
index c49cb1f..8ede590b 100644
--- a/src/egl/drivers/dri2/platform_x11.c
+++ b/src/egl/drivers/dri2/platform_x11.c
@@ -704,7 +704,6 @@ dri2_x11_connect(struct dri2_egl_display *dri2_dpy)

if (dri2_dpy->driver_name == NULL) {
close(dri2_dpy->fd);
- free(dri2_dpy->driver_name);
free(connect);
return EGL_FALSE;
}
--
2.7.4
Eric Engestrom
2017-12-01 15:41:24 UTC
Permalink
Post by v***@gmail.com
In this condition dri2_dpy->driver_name string always equals
NULL, so call to free() is useless
Reviewed and pushed :)

Are you finding all of these by inspection, or are you using a tool?
Post by v***@gmail.com
---
src/egl/drivers/dri2/platform_x11.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/src/egl/drivers/dri2/platform_x11.c b/src/egl/drivers/dri2/platform_x11.c
index c49cb1f..8ede590b 100644
--- a/src/egl/drivers/dri2/platform_x11.c
+++ b/src/egl/drivers/dri2/platform_x11.c
@@ -704,7 +704,6 @@ dri2_x11_connect(struct dri2_egl_display *dri2_dpy)
if (dri2_dpy->driver_name == NULL) {
close(dri2_dpy->fd);
- free(dri2_dpy->driver_name);
free(connect);
return EGL_FALSE;
}
--
2.7.4
Vadym Shovkoplias
2017-12-01 16:03:49 UTC
Permalink
Hi Eric,

Mostly by a static analysis tool. It found at least 7 issues with useless
free() calls and other problems that probably should be fixed.
Suggest please should I create one cumulative commit for this or it should
be a separate commits ?
Post by Eric Engestrom
Post by v***@gmail.com
In this condition dri2_dpy->driver_name string always equals
NULL, so call to free() is useless
Reviewed and pushed :)
Are you finding all of these by inspection, or are you using a tool?
Post by v***@gmail.com
---
src/egl/drivers/dri2/platform_x11.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/src/egl/drivers/dri2/platform_x11.c b/src/egl/drivers/dri2/
platform_x11.c
Post by v***@gmail.com
index c49cb1f..8ede590b 100644
--- a/src/egl/drivers/dri2/platform_x11.c
+++ b/src/egl/drivers/dri2/platform_x11.c
@@ -704,7 +704,6 @@ dri2_x11_connect(struct dri2_egl_display *dri2_dpy)
if (dri2_dpy->driver_name == NULL) {
close(dri2_dpy->fd);
- free(dri2_dpy->driver_name);
free(connect);
return EGL_FALSE;
}
--
2.7.4
--
Vadym Shovkoplias | Software engineer
GlobalLogic
P +x.xxx.xxx.xxxx M +3.8050.931.7304 S vadym.shovkoplias
www.globallogic.com
<http://www.globallogic.com/>
http://www.globallogic.com/email_disclaimer.txt
Vadim Shovkoplias
2017-12-04 10:48:55 UTC
Permalink
Hi Eric,

Mostly by a static analysis tool. It found at least 7 issues with useless
free() calls and other problems that probably should be fixed.
Suggest please should I create one cumulative commit for this or it should
be a separate commits ?
Post by Eric Engestrom
Post by v***@gmail.com
In this condition dri2_dpy->driver_name string always equals
NULL, so call to free() is useless
Reviewed and pushed :)
Are you finding all of these by inspection, or are you using a tool?
Post by v***@gmail.com
---
src/egl/drivers/dri2/platform_x11.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/src/egl/drivers/dri2/platform_x11.c b/src/egl/drivers/dri2/
platform_x11.c
Post by v***@gmail.com
index c49cb1f..8ede590b 100644
--- a/src/egl/drivers/dri2/platform_x11.c
+++ b/src/egl/drivers/dri2/platform_x11.c
@@ -704,7 +704,6 @@ dri2_x11_connect(struct dri2_egl_display *dri2_dpy)
if (dri2_dpy->driver_name == NULL) {
close(dri2_dpy->fd);
- free(dri2_dpy->driver_name);
free(connect);
return EGL_FALSE;
}
--
2.7.4
Eric Engestrom
2017-12-04 16:52:52 UTC
Permalink
Post by Vadym Shovkoplias
Hi Eric,
Hey, sorry, I forgot to hit "send" on the reply I wrote on friday :]
Post by Vadym Shovkoplias
Mostly by a static analysis tool. It found at least 7 issues with useless
free() calls and other problems that probably should be fixed.
What tool? It would be interesting for others to know what tools exist,
especially when they find issues other tools didn't :)
Post by Vadym Shovkoplias
Suggest please should I create one cumulative commit for this or it should
be a separate commits ?
Same kind of issues in the same module should be grouped, whereas
different kind of issues or different modules should be separate.

Don't worry too much about it though, if people ask you to merge or
split commits, it's not that complicated to do for a v2 :)
Post by Vadym Shovkoplias
Post by Eric Engestrom
Post by v***@gmail.com
In this condition dri2_dpy->driver_name string always equals
NULL, so call to free() is useless
Reviewed and pushed :)
Are you finding all of these by inspection, or are you using a tool?
Post by v***@gmail.com
---
src/egl/drivers/dri2/platform_x11.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/src/egl/drivers/dri2/platform_x11.c b/src/egl/drivers/dri2/
platform_x11.c
Post by v***@gmail.com
index c49cb1f..8ede590b 100644
--- a/src/egl/drivers/dri2/platform_x11.c
+++ b/src/egl/drivers/dri2/platform_x11.c
@@ -704,7 +704,6 @@ dri2_x11_connect(struct dri2_egl_display *dri2_dpy)
if (dri2_dpy->driver_name == NULL) {
close(dri2_dpy->fd);
- free(dri2_dpy->driver_name);
free(connect);
return EGL_FALSE;
}
--
2.7.4
Vadym Shovkoplias
2017-12-06 08:27:40 UTC
Permalink
Hi Eric,

I used smatch (http://smatch.sourceforge.net/). It is mainly used for Linux
kernel.
Post by Eric Engestrom
Post by Vadym Shovkoplias
Hi Eric,
Hey, sorry, I forgot to hit "send" on the reply I wrote on friday :]
Post by Vadym Shovkoplias
Mostly by a static analysis tool. It found at least 7 issues with useless
free() calls and other problems that probably should be fixed.
What tool? It would be interesting for others to know what tools exist,
especially when they find issues other tools didn't :)
Post by Vadym Shovkoplias
Suggest please should I create one cumulative commit for this or it
should
Post by Vadym Shovkoplias
be a separate commits ?
Same kind of issues in the same module should be grouped, whereas
different kind of issues or different modules should be separate.
Don't worry too much about it though, if people ask you to merge or
split commits, it's not that complicated to do for a v2 :)
Post by Vadym Shovkoplias
Post by Eric Engestrom
Post by v***@gmail.com
In this condition dri2_dpy->driver_name string always equals
NULL, so call to free() is useless
Reviewed and pushed :)
Are you finding all of these by inspection, or are you using a tool?
Post by v***@gmail.com
---
src/egl/drivers/dri2/platform_x11.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/src/egl/drivers/dri2/platform_x11.c
b/src/egl/drivers/dri2/
Post by Vadym Shovkoplias
Post by Eric Engestrom
platform_x11.c
Post by v***@gmail.com
index c49cb1f..8ede590b 100644
--- a/src/egl/drivers/dri2/platform_x11.c
+++ b/src/egl/drivers/dri2/platform_x11.c
@@ -704,7 +704,6 @@ dri2_x11_connect(struct dri2_egl_display
*dri2_dpy)
Post by Vadym Shovkoplias
Post by Eric Engestrom
Post by v***@gmail.com
if (dri2_dpy->driver_name == NULL) {
close(dri2_dpy->fd);
- free(dri2_dpy->driver_name);
free(connect);
return EGL_FALSE;
}
--
2.7.4
--
Vadym Shovkoplias | Software engineer
GlobalLogic
P +x.xxx.xxx.xxxx M +3.8050.931.7304 S vadym.shovkoplias
www.globallogic.com
<http://www.globallogic.com/>
http://www.globallogic.com/email_disclaimer.txt
Vadim Shovkoplias
2017-12-06 12:48:41 UTC
Permalink
Hi Eric,

I used smatch (http://smatch.sourceforge.net/). It is mainly used for Linux
kernel.
Post by Eric Engestrom
Post by Vadym Shovkoplias
Hi Eric,
Hey, sorry, I forgot to hit "send" on the reply I wrote on friday :]
Post by Vadym Shovkoplias
Mostly by a static analysis tool. It found at least 7 issues with useless
free() calls and other problems that probably should be fixed.
What tool? It would be interesting for others to know what tools exist,
especially when they find issues other tools didn't :)
Post by Vadym Shovkoplias
Suggest please should I create one cumulative commit for this or it
should
Post by Vadym Shovkoplias
be a separate commits ?
Same kind of issues in the same module should be grouped, whereas
different kind of issues or different modules should be separate.
Don't worry too much about it though, if people ask you to merge or
split commits, it's not that complicated to do for a v2 :)
Post by Vadym Shovkoplias
Post by Eric Engestrom
Post by v***@gmail.com
In this condition dri2_dpy->driver_name string always equals
NULL, so call to free() is useless
Reviewed and pushed :)
Are you finding all of these by inspection, or are you using a tool?
Post by v***@gmail.com
---
src/egl/drivers/dri2/platform_x11.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/src/egl/drivers/dri2/platform_x11.c
b/src/egl/drivers/dri2/
Post by Vadym Shovkoplias
Post by Eric Engestrom
platform_x11.c
Post by v***@gmail.com
index c49cb1f..8ede590b 100644
--- a/src/egl/drivers/dri2/platform_x11.c
+++ b/src/egl/drivers/dri2/platform_x11.c
@@ -704,7 +704,6 @@ dri2_x11_connect(struct dri2_egl_display
*dri2_dpy)
Post by Vadym Shovkoplias
Post by Eric Engestrom
Post by v***@gmail.com
if (dri2_dpy->driver_name == NULL) {
close(dri2_dpy->fd);
- free(dri2_dpy->driver_name);
free(connect);
return EGL_FALSE;
}
--
2.7.4
Loading...