On Tuesday, 2018-09-11 15:42:04 +0300, asimiklit.w...@gmail.com wrote:
> From: Andrii Simiklit <andrii.simik...@globallogic.com>
>
> 1. tools/i965_disasm.c:58:4: warning:
> ignoring return value of ‘fread’,
> declared with attribute warn_unused_result
> fread(assembly, *end, 1, fp);
>
> 2. tools/aub_read.c:271:31: warning: unused variable ‘end’
> const uint32_t *p = data, *end = data + data_len, *next;
>
> 3. tools/aub_mem.c:292:13: warning: unused variable ‘res’
> void *res = mmap((uint8_t *)bo.map + map_offset, 4096, PROT_READ,
> tools/aub_mem.c:357:13: warning: unused variable ‘res’
> void *res = mmap((uint8_t *)bo.map + (page - bo.addr), 4096, PROT_READ,
>
> Signed-off-by: Andrii Simiklit <andrii.simik...@globallogic.com>
> ---
> src/intel/tools/aub_mem.c | 10 ++++++----
> src/intel/tools/aub_read.c | 2 +-
> src/intel/tools/i965_disasm.c | 3 ++-
> 3 files changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/src/intel/tools/aub_mem.c b/src/intel/tools/aub_mem.c
> index 58b51b7..98e1421 100644
> --- a/src/intel/tools/aub_mem.c
> +++ b/src/intel/tools/aub_mem.c
> @@ -289,8 +289,9 @@ aub_mem_get_ggtt_bo(void *_mem, uint64_t address)
> continue;
>
> uint32_t map_offset = i->virt_addr - address;
> - void *res = mmap((uint8_t *)bo.map + map_offset, 4096, PROT_READ,
> - MAP_SHARED | MAP_FIXED, mem->mem_fd,
> phys_mem->fd_offset);
> + MAYBE_UNUSED void *res =
> + mmap((uint8_t *)bo.map + map_offset, 4096, PROT_READ,
> + MAP_SHARED | MAP_FIXED, mem->mem_fd, phys_mem->fd_offset);
> assert(res != MAP_FAILED);
> }
>
> @@ -354,8 +355,9 @@ aub_mem_get_ppgtt_bo(void *_mem, uint64_t address)
> for (uint64_t page = address; page < end; page += 4096) {
> struct phys_mem *phys_mem = ppgtt_walk(mem, mem->pml4, page);
>
> - void *res = mmap((uint8_t *)bo.map + (page - bo.addr), 4096, PROT_READ,
> - MAP_SHARED | MAP_FIXED, mem->mem_fd,
> phys_mem->fd_offset);
> + MAYBE_UNUSED void *res =
> + mmap((uint8_t *)bo.map + (page - bo.addr), 4096, PROT_READ,
> + MAP_SHARED | MAP_FIXED, mem->mem_fd, phys_mem->fd_offset);
> assert(res != MAP_FAILED);
> }
>
R-b on the above
> diff --git a/src/intel/tools/aub_read.c b/src/intel/tools/aub_read.c
> index 5b704e8..0a1f84a 100644
> --- a/src/intel/tools/aub_read.c
> +++ b/src/intel/tools/aub_read.c
> @@ -268,7 +268,7 @@ handle_memtrace_mem_write(struct aub_read *read, const
> uint32_t *p)
> int
> aub_read_command(struct aub_read *read, const void *data, uint32_t data_len)
> {
> - const uint32_t *p = data, *end = data + data_len, *next;
> + MAYBE_UNUSED const uint32_t *p = data, *end = data + data_len, *next;
You're flagging all of them as MAYBE_UNUSED; please split out the one
variable for which it applies?
> uint32_t h, header_length, bias;
>
> assert(data_len >= 4);
> diff --git a/src/intel/tools/i965_disasm.c b/src/intel/tools/i965_disasm.c
> index 73a6760..e207def 100644
> --- a/src/intel/tools/i965_disasm.c
> +++ b/src/intel/tools/i965_disasm.c
> @@ -55,7 +55,8 @@ i965_disasm_read_binary(FILE *fp, size_t *end)
> if (assembly == NULL)
> return NULL;
>
> - fread(assembly, *end, 1, fp);
> + MAYBE_UNUSED size_t size = fread(assembly, *end, 1, fp);
> + assert((size == *end) && "error: unable to read all elements!");
Please split this out in a separate patch, and I don't think this
`size == *end` is right
From `man 3 fread`:
> On success, fread() and fwrite() return the number of items read or
> written. This number equals the number of bytes transferred only when
> size is 1.
but here, size is `*end` and nmemb is `1`, so I would expect fread to
return 1 on success and 0 on failure.
Have you tested this? What value did you see?
You can test the complete EOF failure path by adding this just before
fread():
fseek(fp, 0, SEEK_END);
And the more insidious "just a bit too short" failure path with:
fseek(fp, 1, SEEK_CUR);
> fclose(fp);
>
> return assembly;
> --
> 2.7.4
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
