* [PATCH] lib: fix warnings on ignoring return values
@ 2014-10-15 11:43 Mika Kuoppala
2014-10-15 17:09 ` Thomas Wood
0 siblings, 1 reply; 4+ messages in thread
From: Mika Kuoppala @ 2014-10-15 11:43 UTC (permalink / raw)
To: intel-gfx
Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
---
lib/igt_core.c | 21 ++++++++++++++++-----
lib/igt_debugfs.c | 6 +++---
lib/igt_kms.c | 22 +++++++++++++---------
lib/intel_os.c | 2 +-
4 files changed, 33 insertions(+), 18 deletions(-)
diff --git a/lib/igt_core.c b/lib/igt_core.c
index e3d5fb0..287e345 100644
--- a/lib/igt_core.c
+++ b/lib/igt_core.c
@@ -336,13 +336,18 @@ static void low_mem_killer_disable(bool disable)
/* writing 9999 to this module parameter effectively diables the
* low memory killer. This is not a real file, so we dont need to
* seek to the start or truncate it */
- write(fd, no_lowmem_killer, sizeof(no_lowmem_killer));
+ igt_assert(
+ write(fd, no_lowmem_killer, sizeof(no_lowmem_killer))
+ == sizeof(no_lowmem_killer));
close(fd);
} else {
/* just re-enstate the original settings */
fd = open(adj_fname, O_WRONLY);
igt_assert(fd != -1);
- write(fd, prev_adj_scores, adj_scores_len);
+ igt_assert(
+ write(fd, prev_adj_scores, adj_scores_len)
+ == adj_scores_len);
+
close(fd);
}
@@ -775,7 +780,10 @@ void __igt_skip_check(const char *file, const int line,
char *err_str = NULL;
if (err)
- asprintf(&err_str, "Last errno: %i, %s\n", err, strerror(err));
+ igt_assert(
+ asprintf(&err_str, "Last errno: %i, %s\n",
+ err, strerror(err))
+ > 0);
if (f) {
static char *buf;
@@ -785,7 +793,7 @@ void __igt_skip_check(const char *file, const int line,
free(buf);
va_start(args, f);
- vasprintf(&buf, f, args);
+ igt_assert(vasprintf(&buf, f, args) >= 0);
va_end(args);
igt_skip("Test requirement not met in function %s, file %s:%i:\n"
@@ -878,7 +886,10 @@ void __igt_fail_assert(int exitcode, const char *file,
char *err_str = NULL;
if (err)
- asprintf(&err_str, "Last errno: %i, %s\n", err, strerror(err));
+ igt_assert(
+ asprintf(&err_str, "Last errno: %i, %s\n",
+ err, strerror(err))
+ > 0);
printf("Test assertion failure function %s, file %s:%i:\n"
"Failed assertion: %s\n"
diff --git a/lib/igt_debugfs.c b/lib/igt_debugfs.c
index b30f5e4..f4ff61a 100644
--- a/lib/igt_debugfs.c
+++ b/lib/igt_debugfs.c
@@ -297,7 +297,7 @@ static bool igt_pipe_crc_do_start(igt_pipe_crc_t *pipe_crc)
sprintf(buf, "pipe %s %s", kmstest_pipe_name(pipe_crc->pipe),
pipe_crc_source_name(pipe_crc->source));
errno = 0;
- write(pipe_crc->ctl_fd, buf, strlen(buf));
+ igt_assert(write(pipe_crc->ctl_fd, buf, strlen(buf)) == strlen(buf));
if (errno != 0)
return false;
@@ -309,7 +309,7 @@ static void igt_pipe_crc_pipe_off(int fd, enum pipe pipe)
char buf[32];
sprintf(buf, "pipe %s none", kmstest_pipe_name(pipe));
- write(fd, buf, strlen(buf));
+ igt_assert(write(fd, buf, strlen(buf) == strlen(buf)));
}
static void igt_pipe_crc_reset(void)
@@ -446,7 +446,7 @@ void igt_pipe_crc_stop(igt_pipe_crc_t *pipe_crc)
char buf[32];
sprintf(buf, "pipe %s none", kmstest_pipe_name(pipe_crc->pipe));
- write(pipe_crc->ctl_fd, buf, strlen(buf));
+ igt_assert(write(pipe_crc->ctl_fd, buf, strlen(buf)) == strlen(buf));
}
static bool pipe_crc_init_from_string(igt_crc_t *crc, const char *line)
diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index 0547147..49bc6a9 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -323,11 +323,13 @@ static char* get_debugfs_connector_path(int drm_fd, drmModeConnector *connector,
{
char *path;
- asprintf(&path, "/sys/kernel/debug/dri/%d/%s-%d/%s",
- get_card_number(drm_fd),
- kmstest_connector_type_str(connector->connector_type),
- connector->connector_type_id,
- file);
+ igt_assert(
+ asprintf(&path, "/sys/kernel/debug/dri/%d/%s-%d/%s",
+ get_card_number(drm_fd),
+ kmstest_connector_type_str(connector->connector_type),
+ connector->connector_type_id,
+ file)
+ > 0);
return path;
}
@@ -851,9 +853,11 @@ static void igt_output_refresh(igt_output_t *output)
if (!output->name) {
drmModeConnector *c = output->config.connector;
- asprintf(&output->name, "%s-%d",
- kmstest_connector_type_str(c->connector_type),
- c->connector_type_id);
+ igt_assert(
+ asprintf(&output->name, "%s-%d",
+ kmstest_connector_type_str(c->connector_type),
+ c->connector_type_id)
+ > 0);
}
LOG(display, "%s: Selecting pipe %s\n", output->name,
@@ -1795,7 +1799,7 @@ void igt_reset_connectors(void)
for (tmp = forced_connectors; *tmp; tmp++) {
int fd = open(*tmp, O_WRONLY | O_TRUNC);
- write(fd, "unspecified", 11);
+ igt_assert(write(fd, "unspecified", 11) == 11);
close(fd);
}
}
diff --git a/lib/intel_os.c b/lib/intel_os.c
index c8793b9..c05e4b8 100644
--- a/lib/intel_os.c
+++ b/lib/intel_os.c
@@ -257,7 +257,7 @@ intel_purge_vm_caches(void)
if (fd < 0)
return;
- write(fd, "3\n", 2);
+ igt_assert(write(fd, "3\n", 2) == 2);
close(fd);
}
--
1.9.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] lib: fix warnings on ignoring return values
2014-10-15 11:43 [PATCH] lib: fix warnings on ignoring return values Mika Kuoppala
@ 2014-10-15 17:09 ` Thomas Wood
2015-03-25 17:34 ` [PATCH i-g-t] igt.cocci: check the return values of various functions Thomas Wood
0 siblings, 1 reply; 4+ messages in thread
From: Thomas Wood @ 2014-10-15 17:09 UTC (permalink / raw)
To: Mika Kuoppala; +Cc: Intel Graphics Development
On 15 October 2014 12:43, Mika Kuoppala <mika.kuoppala@linux.intel.com> wrote:
> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
> ---
> lib/igt_core.c | 21 ++++++++++++++++-----
> lib/igt_debugfs.c | 6 +++---
> lib/igt_kms.c | 22 +++++++++++++---------
> lib/intel_os.c | 2 +-
> 4 files changed, 33 insertions(+), 18 deletions(-)
>
> diff --git a/lib/igt_core.c b/lib/igt_core.c
> index e3d5fb0..287e345 100644
> --- a/lib/igt_core.c
> +++ b/lib/igt_core.c
> @@ -336,13 +336,18 @@ static void low_mem_killer_disable(bool disable)
> /* writing 9999 to this module parameter effectively diables the
> * low memory killer. This is not a real file, so we dont need to
> * seek to the start or truncate it */
> - write(fd, no_lowmem_killer, sizeof(no_lowmem_killer));
> + igt_assert(
> + write(fd, no_lowmem_killer, sizeof(no_lowmem_killer))
> + == sizeof(no_lowmem_killer));
> close(fd);
> } else {
> /* just re-enstate the original settings */
> fd = open(adj_fname, O_WRONLY);
> igt_assert(fd != -1);
> - write(fd, prev_adj_scores, adj_scores_len);
> + igt_assert(
> + write(fd, prev_adj_scores, adj_scores_len)
> + == adj_scores_len);
> +
These warnings appear on Ubuntu because _FORTIFY_SOURCE is set by
default, which is why they don't always appear in other environments.
The warnings can be disabled by adding -U_FORITIFY_SOURCE to the
compiler flags, or if left enabled it might be worth adding wrappers
for common functions, which can then also be added to igt.cocci.
> close(fd);
> }
>
> @@ -775,7 +780,10 @@ void __igt_skip_check(const char *file, const int line,
> char *err_str = NULL;
>
> if (err)
> - asprintf(&err_str, "Last errno: %i, %s\n", err, strerror(err));
> + igt_assert(
> + asprintf(&err_str, "Last errno: %i, %s\n",
> + err, strerror(err))
> + > 0);
>
> if (f) {
> static char *buf;
> @@ -785,7 +793,7 @@ void __igt_skip_check(const char *file, const int line,
> free(buf);
>
> va_start(args, f);
> - vasprintf(&buf, f, args);
> + igt_assert(vasprintf(&buf, f, args) >= 0);
> va_end(args);
>
> igt_skip("Test requirement not met in function %s, file %s:%i:\n"
> @@ -878,7 +886,10 @@ void __igt_fail_assert(int exitcode, const char *file,
> char *err_str = NULL;
>
> if (err)
> - asprintf(&err_str, "Last errno: %i, %s\n", err, strerror(err));
> + igt_assert(
> + asprintf(&err_str, "Last errno: %i, %s\n",
> + err, strerror(err))
> + > 0);
>
> printf("Test assertion failure function %s, file %s:%i:\n"
> "Failed assertion: %s\n"
> diff --git a/lib/igt_debugfs.c b/lib/igt_debugfs.c
> index b30f5e4..f4ff61a 100644
> --- a/lib/igt_debugfs.c
> +++ b/lib/igt_debugfs.c
> @@ -297,7 +297,7 @@ static bool igt_pipe_crc_do_start(igt_pipe_crc_t *pipe_crc)
> sprintf(buf, "pipe %s %s", kmstest_pipe_name(pipe_crc->pipe),
> pipe_crc_source_name(pipe_crc->source));
> errno = 0;
> - write(pipe_crc->ctl_fd, buf, strlen(buf));
> + igt_assert(write(pipe_crc->ctl_fd, buf, strlen(buf)) == strlen(buf));
> if (errno != 0)
> return false;
>
> @@ -309,7 +309,7 @@ static void igt_pipe_crc_pipe_off(int fd, enum pipe pipe)
> char buf[32];
>
> sprintf(buf, "pipe %s none", kmstest_pipe_name(pipe));
> - write(fd, buf, strlen(buf));
> + igt_assert(write(fd, buf, strlen(buf) == strlen(buf)));
> }
>
> static void igt_pipe_crc_reset(void)
> @@ -446,7 +446,7 @@ void igt_pipe_crc_stop(igt_pipe_crc_t *pipe_crc)
> char buf[32];
>
> sprintf(buf, "pipe %s none", kmstest_pipe_name(pipe_crc->pipe));
> - write(pipe_crc->ctl_fd, buf, strlen(buf));
> + igt_assert(write(pipe_crc->ctl_fd, buf, strlen(buf)) == strlen(buf));
> }
>
> static bool pipe_crc_init_from_string(igt_crc_t *crc, const char *line)
> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> index 0547147..49bc6a9 100644
> --- a/lib/igt_kms.c
> +++ b/lib/igt_kms.c
> @@ -323,11 +323,13 @@ static char* get_debugfs_connector_path(int drm_fd, drmModeConnector *connector,
> {
> char *path;
>
> - asprintf(&path, "/sys/kernel/debug/dri/%d/%s-%d/%s",
> - get_card_number(drm_fd),
> - kmstest_connector_type_str(connector->connector_type),
> - connector->connector_type_id,
> - file);
> + igt_assert(
> + asprintf(&path, "/sys/kernel/debug/dri/%d/%s-%d/%s",
> + get_card_number(drm_fd),
> + kmstest_connector_type_str(connector->connector_type),
> + connector->connector_type_id,
> + file)
> + > 0);
>
> return path;
> }
> @@ -851,9 +853,11 @@ static void igt_output_refresh(igt_output_t *output)
> if (!output->name) {
> drmModeConnector *c = output->config.connector;
>
> - asprintf(&output->name, "%s-%d",
> - kmstest_connector_type_str(c->connector_type),
> - c->connector_type_id);
> + igt_assert(
> + asprintf(&output->name, "%s-%d",
> + kmstest_connector_type_str(c->connector_type),
> + c->connector_type_id)
> + > 0);
> }
>
> LOG(display, "%s: Selecting pipe %s\n", output->name,
> @@ -1795,7 +1799,7 @@ void igt_reset_connectors(void)
>
> for (tmp = forced_connectors; *tmp; tmp++) {
> int fd = open(*tmp, O_WRONLY | O_TRUNC);
> - write(fd, "unspecified", 11);
> + igt_assert(write(fd, "unspecified", 11) == 11);
> close(fd);
> }
> }
> diff --git a/lib/intel_os.c b/lib/intel_os.c
> index c8793b9..c05e4b8 100644
> --- a/lib/intel_os.c
> +++ b/lib/intel_os.c
> @@ -257,7 +257,7 @@ intel_purge_vm_caches(void)
> if (fd < 0)
> return;
>
> - write(fd, "3\n", 2);
> + igt_assert(write(fd, "3\n", 2) == 2);
> close(fd);
> }
>
> --
> 1.9.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH i-g-t] igt.cocci: check the return values of various functions
2014-10-15 17:09 ` Thomas Wood
@ 2015-03-25 17:34 ` Thomas Wood
2015-03-25 18:29 ` Daniel Vetter
0 siblings, 1 reply; 4+ messages in thread
From: Thomas Wood @ 2015-03-25 17:34 UTC (permalink / raw)
To: intel-gfx
Add rules to fix unused-result warnings when compiling with
_FORTIFY_SOURCE defined and apply them to the library and tests.
Signed-off-by: Thomas Wood <thomas.wood@intel.com>
---
lib/igt.cocci | 28 ++++++++++++++++++++++++++++
lib/igt_core.c | 20 ++++++++++++--------
lib/igt_debugfs.c | 6 +++---
lib/igt_kms.c | 15 ++++++---------
lib/intel_os.c | 2 +-
lib/rendercopy_gen8.c | 2 +-
lib/rendercopy_gen9.c | 2 +-
tests/kms_fbc_crc.c | 4 ++--
tests/kms_vblank.c | 2 +-
tests/pm_lpsp.c | 4 ++--
tests/prime_udl.c | 2 +-
11 files changed, 58 insertions(+), 29 deletions(-)
diff --git a/lib/igt.cocci b/lib/igt.cocci
index 7dc398d..156f0cf 100644
--- a/lib/igt.cocci
+++ b/lib/igt.cocci
@@ -173,3 +173,31 @@ int E3, E4;
- igt_assert(E3 < E4);
+ igt_assert_lt(E3, E4);
)
+
+// avoid unused-result warnings when compiling with _FORTIFY_SOURCE defined
+@@
+identifier func =~ "^(read|write)$";
+expression list[2] E;
+expression size;
+@@
+-func(E, size);
++igt_assert_eq(func(E, size), size);
+
+@@
+expression ptr, size, nmemb, stream;
+@@
+-fread(ptr, size, nmemb, stream);
++igt_assert_eq(fread(ptr, size, nmemb, stream), nmemb);
+
+@@
+expression list E;
+@@
+-fgets(E);
++igt_assert_neq(fgets(E), NULL);
+
+@@
+identifier func =~ "^v?asprintf$";
+expression list E;
+@@
+-func(E);
++igt_assert_neq(func(E), -1);
diff --git a/lib/igt_core.c b/lib/igt_core.c
index 7445eab..3a6dc79 100644
--- a/lib/igt_core.c
+++ b/lib/igt_core.c
@@ -412,13 +412,15 @@ static void low_mem_killer_disable(bool disable)
/* writing 9999 to this module parameter effectively diables the
* low memory killer. This is not a real file, so we dont need to
* seek to the start or truncate it */
- write(fd, no_lowmem_killer, sizeof(no_lowmem_killer));
+ igt_assert_eq(write(fd, no_lowmem_killer, sizeof(no_lowmem_killer)),
+ sizeof(no_lowmem_killer));
close(fd);
} else {
/* just re-enstate the original settings */
fd = open(adj_fname, O_WRONLY);
igt_assert(fd != -1);
- write(fd, prev_adj_scores, adj_scores_len);
+ igt_assert_eq(write(fd, prev_adj_scores, adj_scores_len),
+ adj_scores_len);
close(fd);
}
@@ -859,7 +861,8 @@ void __igt_skip_check(const char *file, const int line,
char *err_str = NULL;
if (err)
- asprintf(&err_str, "Last errno: %i, %s\n", err, strerror(err));
+ igt_assert_neq(asprintf(&err_str, "Last errno: %i, %s\n", err, strerror(err)),
+ -1);
if (f) {
static char *buf;
@@ -869,7 +872,7 @@ void __igt_skip_check(const char *file, const int line,
free(buf);
va_start(args, f);
- vasprintf(&buf, f, args);
+ igt_assert_neq(vasprintf(&buf, f, args), -1);
va_end(args);
igt_skip("Test requirement not met in function %s, file %s:%i:\n"
@@ -1396,10 +1399,11 @@ static void fatal_sig_handler(int sig)
for (i = 0; i < ARRAY_SIZE(handled_signals); i++) {
if (handled_signals[i].number == sig) {
- write(STDERR_FILENO, "Received signal ", 16);
- write(STDERR_FILENO, handled_signals[i].name,
- handled_signals[i].name_len);
- write(STDERR_FILENO, ".\n", 2);
+ igt_assert_eq(write(STDERR_FILENO, "Received signal ", 16),
+ 16);
+ igt_assert_eq(write(STDERR_FILENO, handled_signals[i].name, handled_signals[i].name_len),
+ handled_signals[i].name_len);
+ igt_assert_eq(write(STDERR_FILENO, ".\n", 2), 2);
break;
}
}
diff --git a/lib/igt_debugfs.c b/lib/igt_debugfs.c
index 85c3f22..df7f453 100644
--- a/lib/igt_debugfs.c
+++ b/lib/igt_debugfs.c
@@ -273,7 +273,7 @@ static bool igt_pipe_crc_do_start(igt_pipe_crc_t *pipe_crc)
sprintf(buf, "pipe %s %s", kmstest_pipe_name(pipe_crc->pipe),
pipe_crc_source_name(pipe_crc->source));
errno = 0;
- write(pipe_crc->ctl_fd, buf, strlen(buf));
+ igt_assert_eq(write(pipe_crc->ctl_fd, buf, strlen(buf)), strlen(buf));
if (errno != 0)
return false;
@@ -285,7 +285,7 @@ static void igt_pipe_crc_pipe_off(int fd, enum pipe pipe)
char buf[32];
sprintf(buf, "pipe %s none", kmstest_pipe_name(pipe));
- write(fd, buf, strlen(buf));
+ igt_assert_eq(write(fd, buf, strlen(buf)), strlen(buf));
}
static void igt_pipe_crc_reset(void)
@@ -417,7 +417,7 @@ void igt_pipe_crc_stop(igt_pipe_crc_t *pipe_crc)
char buf[32];
sprintf(buf, "pipe %s none", kmstest_pipe_name(pipe_crc->pipe));
- write(pipe_crc->ctl_fd, buf, strlen(buf));
+ igt_assert_eq(write(pipe_crc->ctl_fd, buf, strlen(buf)), strlen(buf));
}
static bool pipe_crc_init_from_string(igt_crc_t *crc, const char *line)
diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index 9c131f0..6cb1f08 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -434,9 +434,8 @@ bool kmstest_force_connector(int drm_fd, drmModeConnector *connector,
break;
}
- asprintf(&path, "%s-%d/force",
- kmstest_connector_type_str(connector->connector_type),
- connector->connector_type_id);
+ igt_assert_neq(asprintf(&path, "%s-%d/force", kmstest_connector_type_str(connector->connector_type), connector->connector_type_id),
+ -1);
debugfs_fd = igt_debugfs_open(path, O_WRONLY | O_TRUNC);
if (debugfs_fd == -1) {
@@ -494,9 +493,8 @@ void kmstest_force_edid(int drm_fd, drmModeConnector *connector,
char *path;
int debugfs_fd, ret;
- asprintf(&path, "%s-%d/edid_override",
- kmstest_connector_type_str(connector->connector_type),
- connector->connector_type_id);
+ igt_assert_neq(asprintf(&path, "%s-%d/edid_override", kmstest_connector_type_str(connector->connector_type), connector->connector_type_id),
+ -1);
debugfs_fd = igt_debugfs_open(path, O_WRONLY | O_TRUNC);
free(path);
@@ -910,9 +908,8 @@ static void igt_output_refresh(igt_output_t *output)
if (!output->name) {
drmModeConnector *c = output->config.connector;
- asprintf(&output->name, "%s-%d",
- kmstest_connector_type_str(c->connector_type),
- c->connector_type_id);
+ igt_assert_neq(asprintf(&output->name, "%s-%d", kmstest_connector_type_str(c->connector_type), c->connector_type_id),
+ -1);
}
LOG(display, "%s: Selecting pipe %s\n", output->name,
diff --git a/lib/intel_os.c b/lib/intel_os.c
index 1badd3e..3321a8d 100644
--- a/lib/intel_os.c
+++ b/lib/intel_os.c
@@ -255,7 +255,7 @@ intel_purge_vm_caches(void)
if (fd < 0)
return;
- write(fd, "3\n", 2);
+ igt_assert_eq(write(fd, "3\n", 2), 2);
close(fd);
}
diff --git a/lib/rendercopy_gen8.c b/lib/rendercopy_gen8.c
index baed762..a7fc2c4 100644
--- a/lib/rendercopy_gen8.c
+++ b/lib/rendercopy_gen8.c
@@ -30,7 +30,7 @@
static void dump_batch(struct intel_batchbuffer *batch) {
int fd = open("/tmp/i965-batchbuffers.dump", O_WRONLY | O_CREAT, 0666);
if (fd != -1) {
- write(fd, batch->buffer, 4096);
+ igt_assert_eq(write(fd, batch->buffer, 4096), 4096);
fd = close(fd);
}
}
diff --git a/lib/rendercopy_gen9.c b/lib/rendercopy_gen9.c
index b7b133c..0766192 100644
--- a/lib/rendercopy_gen9.c
+++ b/lib/rendercopy_gen9.c
@@ -31,7 +31,7 @@
static void dump_batch(struct intel_batchbuffer *batch) {
int fd = open("/tmp/i965-batchbuffers.dump", O_WRONLY | O_CREAT, 0666);
if (fd != -1) {
- write(fd, batch->buffer, 4096);
+ igt_assert_eq(write(fd, batch->buffer, 4096), 4096);
fd = close(fd);
}
}
diff --git a/tests/kms_fbc_crc.c b/tests/kms_fbc_crc.c
index 4256fed..10e1ca4 100644
--- a/tests/kms_fbc_crc.c
+++ b/tests/kms_fbc_crc.c
@@ -198,7 +198,7 @@ static bool fbc_enabled(data_t *data)
status = igt_debugfs_fopen("i915_fbc_status", "r");
igt_assert(status);
- fread(str, sizeof(str) - 1, 1, status);
+ igt_assert_eq(fread(str, sizeof(str) - 1, 1, status), 1);
fclose(status);
return strstr(str, "FBC enabled") != NULL;
}
@@ -510,7 +510,7 @@ igt_main
status = igt_debugfs_fopen("i915_fbc_status", "r");
igt_require_f(status, "No i915_fbc_status found\n");
- fread(buf, sizeof(buf), 1, status);
+ igt_assert_eq(fread(buf, sizeof(buf), 1, status), sizeof(buf));
fclose(status);
buf[sizeof(buf) - 1] = '\0';
igt_require_f(!strstr(buf, "unsupported by this chipset") &&
diff --git a/tests/kms_vblank.c b/tests/kms_vblank.c
index 569390e..5a00ccc 100644
--- a/tests/kms_vblank.c
+++ b/tests/kms_vblank.c
@@ -101,7 +101,7 @@ static void query(int fd, bool busy)
busy ? "busy" : "idle", elapsed(&start, &end, count));
if (busy)
- read(fd, buf, sizeof(buf));
+ igt_assert_eq(read(fd, buf, sizeof(buf)), sizeof(buf));
}
igt_main
diff --git a/tests/pm_lpsp.c b/tests/pm_lpsp.c
index 50b3dd9..3ed4c78 100644
--- a/tests/pm_lpsp.c
+++ b/tests/pm_lpsp.c
@@ -45,12 +45,12 @@ static void disable_audio_runtime_pm(void)
fd = open("/sys/module/snd_hda_intel/parameters/power_save", O_WRONLY);
if (fd >= 0) {
- write(fd, "1\n", 2);
+ igt_assert_eq(write(fd, "1\n", 2), 2);
close(fd);
}
fd = open("/sys/bus/pci/devices/0000:00:03.0/power/control", O_WRONLY);
if (fd >= 0) {
- write(fd, "auto\n", 5);
+ igt_assert_eq(write(fd, "auto\n", 5), 5);
close(fd);
}
/* Give some time for it to react. */
diff --git a/tests/prime_udl.c b/tests/prime_udl.c
index 62b381a..d03aee0 100644
--- a/tests/prime_udl.c
+++ b/tests/prime_udl.c
@@ -61,7 +61,7 @@ static int find_and_open_devices(void)
if (!fl)
break;
- fgets(vendor_id, 8, fl);
+ igt_assert_neq(fgets(vendor_id, 8, fl), NULL);
fclose(fl);
venid = strtoul(vendor_id, NULL, 16);
--
2.1.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH i-g-t] igt.cocci: check the return values of various functions
2015-03-25 17:34 ` [PATCH i-g-t] igt.cocci: check the return values of various functions Thomas Wood
@ 2015-03-25 18:29 ` Daniel Vetter
0 siblings, 0 replies; 4+ messages in thread
From: Daniel Vetter @ 2015-03-25 18:29 UTC (permalink / raw)
To: Thomas Wood; +Cc: intel-gfx
On Wed, Mar 25, 2015 at 05:34:26PM +0000, Thomas Wood wrote:
> Add rules to fix unused-result warnings when compiling with
> _FORTIFY_SOURCE defined and apply them to the library and tests.
>
> Signed-off-by: Thomas Wood <thomas.wood@intel.com>
I wasn't sure whether this is really worth the trouble. But since you've
done the job already and we should be able to keep up with cocci this has
my ack.
-Daniel
> ---
> lib/igt.cocci | 28 ++++++++++++++++++++++++++++
> lib/igt_core.c | 20 ++++++++++++--------
> lib/igt_debugfs.c | 6 +++---
> lib/igt_kms.c | 15 ++++++---------
> lib/intel_os.c | 2 +-
> lib/rendercopy_gen8.c | 2 +-
> lib/rendercopy_gen9.c | 2 +-
> tests/kms_fbc_crc.c | 4 ++--
> tests/kms_vblank.c | 2 +-
> tests/pm_lpsp.c | 4 ++--
> tests/prime_udl.c | 2 +-
> 11 files changed, 58 insertions(+), 29 deletions(-)
>
> diff --git a/lib/igt.cocci b/lib/igt.cocci
> index 7dc398d..156f0cf 100644
> --- a/lib/igt.cocci
> +++ b/lib/igt.cocci
> @@ -173,3 +173,31 @@ int E3, E4;
> - igt_assert(E3 < E4);
> + igt_assert_lt(E3, E4);
> )
> +
> +// avoid unused-result warnings when compiling with _FORTIFY_SOURCE defined
> +@@
> +identifier func =~ "^(read|write)$";
> +expression list[2] E;
> +expression size;
> +@@
> +-func(E, size);
> ++igt_assert_eq(func(E, size), size);
> +
> +@@
> +expression ptr, size, nmemb, stream;
> +@@
> +-fread(ptr, size, nmemb, stream);
> ++igt_assert_eq(fread(ptr, size, nmemb, stream), nmemb);
> +
> +@@
> +expression list E;
> +@@
> +-fgets(E);
> ++igt_assert_neq(fgets(E), NULL);
> +
> +@@
> +identifier func =~ "^v?asprintf$";
> +expression list E;
> +@@
> +-func(E);
> ++igt_assert_neq(func(E), -1);
> diff --git a/lib/igt_core.c b/lib/igt_core.c
> index 7445eab..3a6dc79 100644
> --- a/lib/igt_core.c
> +++ b/lib/igt_core.c
> @@ -412,13 +412,15 @@ static void low_mem_killer_disable(bool disable)
> /* writing 9999 to this module parameter effectively diables the
> * low memory killer. This is not a real file, so we dont need to
> * seek to the start or truncate it */
> - write(fd, no_lowmem_killer, sizeof(no_lowmem_killer));
> + igt_assert_eq(write(fd, no_lowmem_killer, sizeof(no_lowmem_killer)),
> + sizeof(no_lowmem_killer));
> close(fd);
> } else {
> /* just re-enstate the original settings */
> fd = open(adj_fname, O_WRONLY);
> igt_assert(fd != -1);
> - write(fd, prev_adj_scores, adj_scores_len);
> + igt_assert_eq(write(fd, prev_adj_scores, adj_scores_len),
> + adj_scores_len);
> close(fd);
> }
>
> @@ -859,7 +861,8 @@ void __igt_skip_check(const char *file, const int line,
> char *err_str = NULL;
>
> if (err)
> - asprintf(&err_str, "Last errno: %i, %s\n", err, strerror(err));
> + igt_assert_neq(asprintf(&err_str, "Last errno: %i, %s\n", err, strerror(err)),
> + -1);
>
> if (f) {
> static char *buf;
> @@ -869,7 +872,7 @@ void __igt_skip_check(const char *file, const int line,
> free(buf);
>
> va_start(args, f);
> - vasprintf(&buf, f, args);
> + igt_assert_neq(vasprintf(&buf, f, args), -1);
> va_end(args);
>
> igt_skip("Test requirement not met in function %s, file %s:%i:\n"
> @@ -1396,10 +1399,11 @@ static void fatal_sig_handler(int sig)
>
> for (i = 0; i < ARRAY_SIZE(handled_signals); i++) {
> if (handled_signals[i].number == sig) {
> - write(STDERR_FILENO, "Received signal ", 16);
> - write(STDERR_FILENO, handled_signals[i].name,
> - handled_signals[i].name_len);
> - write(STDERR_FILENO, ".\n", 2);
> + igt_assert_eq(write(STDERR_FILENO, "Received signal ", 16),
> + 16);
> + igt_assert_eq(write(STDERR_FILENO, handled_signals[i].name, handled_signals[i].name_len),
> + handled_signals[i].name_len);
> + igt_assert_eq(write(STDERR_FILENO, ".\n", 2), 2);
> break;
> }
> }
> diff --git a/lib/igt_debugfs.c b/lib/igt_debugfs.c
> index 85c3f22..df7f453 100644
> --- a/lib/igt_debugfs.c
> +++ b/lib/igt_debugfs.c
> @@ -273,7 +273,7 @@ static bool igt_pipe_crc_do_start(igt_pipe_crc_t *pipe_crc)
> sprintf(buf, "pipe %s %s", kmstest_pipe_name(pipe_crc->pipe),
> pipe_crc_source_name(pipe_crc->source));
> errno = 0;
> - write(pipe_crc->ctl_fd, buf, strlen(buf));
> + igt_assert_eq(write(pipe_crc->ctl_fd, buf, strlen(buf)), strlen(buf));
> if (errno != 0)
> return false;
>
> @@ -285,7 +285,7 @@ static void igt_pipe_crc_pipe_off(int fd, enum pipe pipe)
> char buf[32];
>
> sprintf(buf, "pipe %s none", kmstest_pipe_name(pipe));
> - write(fd, buf, strlen(buf));
> + igt_assert_eq(write(fd, buf, strlen(buf)), strlen(buf));
> }
>
> static void igt_pipe_crc_reset(void)
> @@ -417,7 +417,7 @@ void igt_pipe_crc_stop(igt_pipe_crc_t *pipe_crc)
> char buf[32];
>
> sprintf(buf, "pipe %s none", kmstest_pipe_name(pipe_crc->pipe));
> - write(pipe_crc->ctl_fd, buf, strlen(buf));
> + igt_assert_eq(write(pipe_crc->ctl_fd, buf, strlen(buf)), strlen(buf));
> }
>
> static bool pipe_crc_init_from_string(igt_crc_t *crc, const char *line)
> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> index 9c131f0..6cb1f08 100644
> --- a/lib/igt_kms.c
> +++ b/lib/igt_kms.c
> @@ -434,9 +434,8 @@ bool kmstest_force_connector(int drm_fd, drmModeConnector *connector,
> break;
> }
>
> - asprintf(&path, "%s-%d/force",
> - kmstest_connector_type_str(connector->connector_type),
> - connector->connector_type_id);
> + igt_assert_neq(asprintf(&path, "%s-%d/force", kmstest_connector_type_str(connector->connector_type), connector->connector_type_id),
> + -1);
> debugfs_fd = igt_debugfs_open(path, O_WRONLY | O_TRUNC);
>
> if (debugfs_fd == -1) {
> @@ -494,9 +493,8 @@ void kmstest_force_edid(int drm_fd, drmModeConnector *connector,
> char *path;
> int debugfs_fd, ret;
>
> - asprintf(&path, "%s-%d/edid_override",
> - kmstest_connector_type_str(connector->connector_type),
> - connector->connector_type_id);
> + igt_assert_neq(asprintf(&path, "%s-%d/edid_override", kmstest_connector_type_str(connector->connector_type), connector->connector_type_id),
> + -1);
> debugfs_fd = igt_debugfs_open(path, O_WRONLY | O_TRUNC);
> free(path);
>
> @@ -910,9 +908,8 @@ static void igt_output_refresh(igt_output_t *output)
> if (!output->name) {
> drmModeConnector *c = output->config.connector;
>
> - asprintf(&output->name, "%s-%d",
> - kmstest_connector_type_str(c->connector_type),
> - c->connector_type_id);
> + igt_assert_neq(asprintf(&output->name, "%s-%d", kmstest_connector_type_str(c->connector_type), c->connector_type_id),
> + -1);
> }
>
> LOG(display, "%s: Selecting pipe %s\n", output->name,
> diff --git a/lib/intel_os.c b/lib/intel_os.c
> index 1badd3e..3321a8d 100644
> --- a/lib/intel_os.c
> +++ b/lib/intel_os.c
> @@ -255,7 +255,7 @@ intel_purge_vm_caches(void)
> if (fd < 0)
> return;
>
> - write(fd, "3\n", 2);
> + igt_assert_eq(write(fd, "3\n", 2), 2);
> close(fd);
> }
>
> diff --git a/lib/rendercopy_gen8.c b/lib/rendercopy_gen8.c
> index baed762..a7fc2c4 100644
> --- a/lib/rendercopy_gen8.c
> +++ b/lib/rendercopy_gen8.c
> @@ -30,7 +30,7 @@
> static void dump_batch(struct intel_batchbuffer *batch) {
> int fd = open("/tmp/i965-batchbuffers.dump", O_WRONLY | O_CREAT, 0666);
> if (fd != -1) {
> - write(fd, batch->buffer, 4096);
> + igt_assert_eq(write(fd, batch->buffer, 4096), 4096);
> fd = close(fd);
> }
> }
> diff --git a/lib/rendercopy_gen9.c b/lib/rendercopy_gen9.c
> index b7b133c..0766192 100644
> --- a/lib/rendercopy_gen9.c
> +++ b/lib/rendercopy_gen9.c
> @@ -31,7 +31,7 @@
> static void dump_batch(struct intel_batchbuffer *batch) {
> int fd = open("/tmp/i965-batchbuffers.dump", O_WRONLY | O_CREAT, 0666);
> if (fd != -1) {
> - write(fd, batch->buffer, 4096);
> + igt_assert_eq(write(fd, batch->buffer, 4096), 4096);
> fd = close(fd);
> }
> }
> diff --git a/tests/kms_fbc_crc.c b/tests/kms_fbc_crc.c
> index 4256fed..10e1ca4 100644
> --- a/tests/kms_fbc_crc.c
> +++ b/tests/kms_fbc_crc.c
> @@ -198,7 +198,7 @@ static bool fbc_enabled(data_t *data)
> status = igt_debugfs_fopen("i915_fbc_status", "r");
> igt_assert(status);
>
> - fread(str, sizeof(str) - 1, 1, status);
> + igt_assert_eq(fread(str, sizeof(str) - 1, 1, status), 1);
> fclose(status);
> return strstr(str, "FBC enabled") != NULL;
> }
> @@ -510,7 +510,7 @@ igt_main
>
> status = igt_debugfs_fopen("i915_fbc_status", "r");
> igt_require_f(status, "No i915_fbc_status found\n");
> - fread(buf, sizeof(buf), 1, status);
> + igt_assert_eq(fread(buf, sizeof(buf), 1, status), sizeof(buf));
> fclose(status);
> buf[sizeof(buf) - 1] = '\0';
> igt_require_f(!strstr(buf, "unsupported by this chipset") &&
> diff --git a/tests/kms_vblank.c b/tests/kms_vblank.c
> index 569390e..5a00ccc 100644
> --- a/tests/kms_vblank.c
> +++ b/tests/kms_vblank.c
> @@ -101,7 +101,7 @@ static void query(int fd, bool busy)
> busy ? "busy" : "idle", elapsed(&start, &end, count));
>
> if (busy)
> - read(fd, buf, sizeof(buf));
> + igt_assert_eq(read(fd, buf, sizeof(buf)), sizeof(buf));
> }
>
> igt_main
> diff --git a/tests/pm_lpsp.c b/tests/pm_lpsp.c
> index 50b3dd9..3ed4c78 100644
> --- a/tests/pm_lpsp.c
> +++ b/tests/pm_lpsp.c
> @@ -45,12 +45,12 @@ static void disable_audio_runtime_pm(void)
>
> fd = open("/sys/module/snd_hda_intel/parameters/power_save", O_WRONLY);
> if (fd >= 0) {
> - write(fd, "1\n", 2);
> + igt_assert_eq(write(fd, "1\n", 2), 2);
> close(fd);
> }
> fd = open("/sys/bus/pci/devices/0000:00:03.0/power/control", O_WRONLY);
> if (fd >= 0) {
> - write(fd, "auto\n", 5);
> + igt_assert_eq(write(fd, "auto\n", 5), 5);
> close(fd);
> }
> /* Give some time for it to react. */
> diff --git a/tests/prime_udl.c b/tests/prime_udl.c
> index 62b381a..d03aee0 100644
> --- a/tests/prime_udl.c
> +++ b/tests/prime_udl.c
> @@ -61,7 +61,7 @@ static int find_and_open_devices(void)
> if (!fl)
> break;
>
> - fgets(vendor_id, 8, fl);
> + igt_assert_neq(fgets(vendor_id, 8, fl), NULL);
> fclose(fl);
>
> venid = strtoul(vendor_id, NULL, 16);
> --
> 2.1.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2015-03-25 18:27 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-10-15 11:43 [PATCH] lib: fix warnings on ignoring return values Mika Kuoppala
2014-10-15 17:09 ` Thomas Wood
2015-03-25 17:34 ` [PATCH i-g-t] igt.cocci: check the return values of various functions Thomas Wood
2015-03-25 18:29 ` Daniel Vetter
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.