Re: [PATCH i-g-t] igt.cocci: check the return values of various functions

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxx>

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@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx





[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux