Re: [Mesa-dev] [PATCH v4 5/5] intel/tools: make sure the binary file is properly read
On Tuesday, 2018-12-04 15:36:28 +0200, andrey simiklit wrote: > Hello, > > I'm sorry for disturbing, could I ask you to push this patch if it > possible? ))) Sorry yes, thanks for the reminder :) I just pushed it! > > Regards, > Andrii. > > On Fri, Nov 30, 2018 at 6:19 PM andrey simiklit > wrote: > > > On Fri, Nov 30, 2018 at 5:49 PM Eric Engestrom > > wrote: > > > >> On Friday, 2018-11-30 15:47:11 +, Lionel Landwerlin wrote: > >> > On 14/11/2018 16:30, asimiklit.w...@gmail.com wrote: > >> > > From: Andrii Simiklit > >> > > > >> > > 1. tools/i965_disasm.c:58:4: warning: > >> > > ignoring return value of ‘fread’, > >> > > declared with attribute warn_unused_result > >> > > fread(assembly, *end, 1, fp); > >> > > > >> > > v2: Fixed incorrect return value check. > >> > > ( Eric Engestrom ) > >> > > > >> > > v3: Zero size file check placed before fread with exit() > >> > > ( Eric Engestrom ) > >> > > > >> > > v4: - Title is changed. > >> > > - The 'size' variable was moved to top of a function scope. > >> > > - The assertion was replaced by the proper error handling. > >> > > - The error message on a caller side was fixed. > >> > > ( Eric Engestrom ) > >> > > > >> > > Signed-off-by: Andrii Simiklit > >> > > >> > > >> > With the nit below : > >> > > >> > > >> > Reviewed-by: Lionel Landwerlin > >> > >> I'll change that as I push it in a minute :) > >> Reviewed-by: Eric Engestrom > >> > > > > Thanks a lot for reviews :) > > > > > >> > >> > > >> > > >> > > --- > >> > > src/intel/tools/i965_disasm.c | 16 +--- > >> > > 1 file changed, 13 insertions(+), 3 deletions(-) > >> > > > >> > > diff --git a/src/intel/tools/i965_disasm.c > >> b/src/intel/tools/i965_disasm.c > >> > > index 73a6760fc1..0efbdab706 100644 > >> > > --- a/src/intel/tools/i965_disasm.c > >> > > +++ b/src/intel/tools/i965_disasm.c > >> > > @@ -47,17 +47,23 @@ i965_disasm_get_file_size(FILE *fp) > >> > > static void * > >> > > i965_disasm_read_binary(FILE *fp, size_t *end) > >> > > { > >> > > + size_t size; > >> > > void *assembly; > >> > > *end = i965_disasm_get_file_size(fp); > >> > > + if (!*end) > >> > > + return NULL; > >> > > assembly = malloc(*end + 1); > >> > > if (assembly == NULL) > >> > > return NULL; > >> > > - fread(assembly, *end, 1, fp); > >> > > + size = fread(assembly, *end, 1, fp); > >> > > fclose(fp); > >> > > - > >> > > + if (!size) { > >> > > + free(assembly); > >> > > + return NULL; > >> > > + } > >> > > return assembly; > >> > > } > >> > > @@ -167,7 +173,11 @@ int main(int argc, char *argv[]) > >> > > assembly = i965_disasm_read_binary(fp, ); > >> > > if (!assembly) { > >> > > - fprintf(stderr, "Unable to allocate buffer to read binary > >> file\n"); > >> > > + if(end) > >> > if (end) > >> > > +fprintf(stderr, "Unable to allocate buffer to read binary > >> file\n"); > >> > > + else > >> > > +fprintf(stderr, "Input file is empty\n"); > >> > > + > >> > > exit(EXIT_FAILURE); > >> > > } > >> > > >> > > >> ___ > >> 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
Re: [Mesa-dev] [PATCH v4 5/5] intel/tools: make sure the binary file is properly read
Hello, I'm sorry for disturbing, could I ask you to push this patch if it possible? ))) Regards, Andrii. On Fri, Nov 30, 2018 at 6:19 PM andrey simiklit wrote: > On Fri, Nov 30, 2018 at 5:49 PM Eric Engestrom > wrote: > >> On Friday, 2018-11-30 15:47:11 +, Lionel Landwerlin wrote: >> > On 14/11/2018 16:30, asimiklit.w...@gmail.com wrote: >> > > From: Andrii Simiklit >> > > >> > > 1. tools/i965_disasm.c:58:4: warning: >> > > ignoring return value of ‘fread’, >> > > declared with attribute warn_unused_result >> > > fread(assembly, *end, 1, fp); >> > > >> > > v2: Fixed incorrect return value check. >> > > ( Eric Engestrom ) >> > > >> > > v3: Zero size file check placed before fread with exit() >> > > ( Eric Engestrom ) >> > > >> > > v4: - Title is changed. >> > > - The 'size' variable was moved to top of a function scope. >> > > - The assertion was replaced by the proper error handling. >> > > - The error message on a caller side was fixed. >> > > ( Eric Engestrom ) >> > > >> > > Signed-off-by: Andrii Simiklit >> > >> > >> > With the nit below : >> > >> > >> > Reviewed-by: Lionel Landwerlin >> >> I'll change that as I push it in a minute :) >> Reviewed-by: Eric Engestrom >> > > Thanks a lot for reviews :) > > >> >> > >> > >> > > --- >> > > src/intel/tools/i965_disasm.c | 16 +--- >> > > 1 file changed, 13 insertions(+), 3 deletions(-) >> > > >> > > diff --git a/src/intel/tools/i965_disasm.c >> b/src/intel/tools/i965_disasm.c >> > > index 73a6760fc1..0efbdab706 100644 >> > > --- a/src/intel/tools/i965_disasm.c >> > > +++ b/src/intel/tools/i965_disasm.c >> > > @@ -47,17 +47,23 @@ i965_disasm_get_file_size(FILE *fp) >> > > static void * >> > > i965_disasm_read_binary(FILE *fp, size_t *end) >> > > { >> > > + size_t size; >> > > void *assembly; >> > > *end = i965_disasm_get_file_size(fp); >> > > + if (!*end) >> > > + return NULL; >> > > assembly = malloc(*end + 1); >> > > if (assembly == NULL) >> > > return NULL; >> > > - fread(assembly, *end, 1, fp); >> > > + size = fread(assembly, *end, 1, fp); >> > > fclose(fp); >> > > - >> > > + if (!size) { >> > > + free(assembly); >> > > + return NULL; >> > > + } >> > > return assembly; >> > > } >> > > @@ -167,7 +173,11 @@ int main(int argc, char *argv[]) >> > > assembly = i965_disasm_read_binary(fp, ); >> > > if (!assembly) { >> > > - fprintf(stderr, "Unable to allocate buffer to read binary >> file\n"); >> > > + if(end) >> > if (end) >> > > +fprintf(stderr, "Unable to allocate buffer to read binary >> file\n"); >> > > + else >> > > +fprintf(stderr, "Input file is empty\n"); >> > > + >> > > exit(EXIT_FAILURE); >> > > } >> > >> > >> ___ >> 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
Re: [Mesa-dev] [PATCH v4 5/5] intel/tools: make sure the binary file is properly read
On Fri, Nov 30, 2018 at 5:49 PM Eric Engestrom wrote: > On Friday, 2018-11-30 15:47:11 +, Lionel Landwerlin wrote: > > On 14/11/2018 16:30, asimiklit.w...@gmail.com wrote: > > > From: Andrii Simiklit > > > > > > 1. tools/i965_disasm.c:58:4: warning: > > > ignoring return value of ‘fread’, > > > declared with attribute warn_unused_result > > > fread(assembly, *end, 1, fp); > > > > > > v2: Fixed incorrect return value check. > > > ( Eric Engestrom ) > > > > > > v3: Zero size file check placed before fread with exit() > > > ( Eric Engestrom ) > > > > > > v4: - Title is changed. > > > - The 'size' variable was moved to top of a function scope. > > > - The assertion was replaced by the proper error handling. > > > - The error message on a caller side was fixed. > > > ( Eric Engestrom ) > > > > > > Signed-off-by: Andrii Simiklit > > > > > > With the nit below : > > > > > > Reviewed-by: Lionel Landwerlin > > I'll change that as I push it in a minute :) > Reviewed-by: Eric Engestrom > Thanks a lot for reviews :) > > > > > > > > --- > > > src/intel/tools/i965_disasm.c | 16 +--- > > > 1 file changed, 13 insertions(+), 3 deletions(-) > > > > > > diff --git a/src/intel/tools/i965_disasm.c > b/src/intel/tools/i965_disasm.c > > > index 73a6760fc1..0efbdab706 100644 > > > --- a/src/intel/tools/i965_disasm.c > > > +++ b/src/intel/tools/i965_disasm.c > > > @@ -47,17 +47,23 @@ i965_disasm_get_file_size(FILE *fp) > > > static void * > > > i965_disasm_read_binary(FILE *fp, size_t *end) > > > { > > > + size_t size; > > > void *assembly; > > > *end = i965_disasm_get_file_size(fp); > > > + if (!*end) > > > + return NULL; > > > assembly = malloc(*end + 1); > > > if (assembly == NULL) > > > return NULL; > > > - fread(assembly, *end, 1, fp); > > > + size = fread(assembly, *end, 1, fp); > > > fclose(fp); > > > - > > > + if (!size) { > > > + free(assembly); > > > + return NULL; > > > + } > > > return assembly; > > > } > > > @@ -167,7 +173,11 @@ int main(int argc, char *argv[]) > > > assembly = i965_disasm_read_binary(fp, ); > > > if (!assembly) { > > > - fprintf(stderr, "Unable to allocate buffer to read binary > file\n"); > > > + if(end) > > if (end) > > > +fprintf(stderr, "Unable to allocate buffer to read binary > file\n"); > > > + else > > > +fprintf(stderr, "Input file is empty\n"); > > > + > > > exit(EXIT_FAILURE); > > > } > > > > > ___ > 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
Re: [Mesa-dev] [PATCH v4 5/5] intel/tools: make sure the binary file is properly read
On Friday, 2018-11-30 15:47:11 +, Lionel Landwerlin wrote: > On 14/11/2018 16:30, asimiklit.w...@gmail.com wrote: > > From: Andrii Simiklit > > > > 1. tools/i965_disasm.c:58:4: warning: > > ignoring return value of ‘fread’, > > declared with attribute warn_unused_result > > fread(assembly, *end, 1, fp); > > > > v2: Fixed incorrect return value check. > > ( Eric Engestrom ) > > > > v3: Zero size file check placed before fread with exit() > > ( Eric Engestrom ) > > > > v4: - Title is changed. > > - The 'size' variable was moved to top of a function scope. > > - The assertion was replaced by the proper error handling. > > - The error message on a caller side was fixed. > > ( Eric Engestrom ) > > > > Signed-off-by: Andrii Simiklit > > > With the nit below : > > > Reviewed-by: Lionel Landwerlin I'll change that as I push it in a minute :) Reviewed-by: Eric Engestrom > > > > --- > > src/intel/tools/i965_disasm.c | 16 +--- > > 1 file changed, 13 insertions(+), 3 deletions(-) > > > > diff --git a/src/intel/tools/i965_disasm.c b/src/intel/tools/i965_disasm.c > > index 73a6760fc1..0efbdab706 100644 > > --- a/src/intel/tools/i965_disasm.c > > +++ b/src/intel/tools/i965_disasm.c > > @@ -47,17 +47,23 @@ i965_disasm_get_file_size(FILE *fp) > > static void * > > i965_disasm_read_binary(FILE *fp, size_t *end) > > { > > + size_t size; > > void *assembly; > > *end = i965_disasm_get_file_size(fp); > > + if (!*end) > > + return NULL; > > assembly = malloc(*end + 1); > > if (assembly == NULL) > > return NULL; > > - fread(assembly, *end, 1, fp); > > + size = fread(assembly, *end, 1, fp); > > fclose(fp); > > - > > + if (!size) { > > + free(assembly); > > + return NULL; > > + } > > return assembly; > > } > > @@ -167,7 +173,11 @@ int main(int argc, char *argv[]) > > assembly = i965_disasm_read_binary(fp, ); > > if (!assembly) { > > - fprintf(stderr, "Unable to allocate buffer to read binary file\n"); > > + if(end) > if (end) > > +fprintf(stderr, "Unable to allocate buffer to read binary file\n"); > > + else > > +fprintf(stderr, "Input file is empty\n"); > > + > > exit(EXIT_FAILURE); > > } > > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v4 5/5] intel/tools: make sure the binary file is properly read
On 14/11/2018 16:30, asimiklit.w...@gmail.com wrote: From: Andrii Simiklit 1. tools/i965_disasm.c:58:4: warning: ignoring return value of ‘fread’, declared with attribute warn_unused_result fread(assembly, *end, 1, fp); v2: Fixed incorrect return value check. ( Eric Engestrom ) v3: Zero size file check placed before fread with exit() ( Eric Engestrom ) v4: - Title is changed. - The 'size' variable was moved to top of a function scope. - The assertion was replaced by the proper error handling. - The error message on a caller side was fixed. ( Eric Engestrom ) Signed-off-by: Andrii Simiklit With the nit below : Reviewed-by: Lionel Landwerlin --- src/intel/tools/i965_disasm.c | 16 +--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/src/intel/tools/i965_disasm.c b/src/intel/tools/i965_disasm.c index 73a6760fc1..0efbdab706 100644 --- a/src/intel/tools/i965_disasm.c +++ b/src/intel/tools/i965_disasm.c @@ -47,17 +47,23 @@ i965_disasm_get_file_size(FILE *fp) static void * i965_disasm_read_binary(FILE *fp, size_t *end) { + size_t size; void *assembly; *end = i965_disasm_get_file_size(fp); + if (!*end) + return NULL; assembly = malloc(*end + 1); if (assembly == NULL) return NULL; - fread(assembly, *end, 1, fp); + size = fread(assembly, *end, 1, fp); fclose(fp); - + if (!size) { + free(assembly); + return NULL; + } return assembly; } @@ -167,7 +173,11 @@ int main(int argc, char *argv[]) assembly = i965_disasm_read_binary(fp, ); if (!assembly) { - fprintf(stderr, "Unable to allocate buffer to read binary file\n"); + if(end) if (end) +fprintf(stderr, "Unable to allocate buffer to read binary file\n"); + else +fprintf(stderr, "Input file is empty\n"); + exit(EXIT_FAILURE); } ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v4 5/5] intel/tools: make sure the binary file is properly read
Hello, Could you please take a look on this v4 if it is possible? Thanks, Andrii. On Wed, Nov 14, 2018 at 6:30 PM wrote: > From: Andrii Simiklit > > 1. tools/i965_disasm.c:58:4: warning: > ignoring return value of ‘fread’, > declared with attribute warn_unused_result > fread(assembly, *end, 1, fp); > > v2: Fixed incorrect return value check. >( Eric Engestrom ) > > v3: Zero size file check placed before fread with exit() >( Eric Engestrom ) > > v4: - Title is changed. > - The 'size' variable was moved to top of a function scope. > - The assertion was replaced by the proper error handling. > - The error message on a caller side was fixed. >( Eric Engestrom ) > > Signed-off-by: Andrii Simiklit > --- > src/intel/tools/i965_disasm.c | 16 +--- > 1 file changed, 13 insertions(+), 3 deletions(-) > > diff --git a/src/intel/tools/i965_disasm.c b/src/intel/tools/i965_disasm.c > index 73a6760fc1..0efbdab706 100644 > --- a/src/intel/tools/i965_disasm.c > +++ b/src/intel/tools/i965_disasm.c > @@ -47,17 +47,23 @@ i965_disasm_get_file_size(FILE *fp) > static void * > i965_disasm_read_binary(FILE *fp, size_t *end) > { > + size_t size; > void *assembly; > > *end = i965_disasm_get_file_size(fp); > + if (!*end) > + return NULL; > > assembly = malloc(*end + 1); > if (assembly == NULL) >return NULL; > > - fread(assembly, *end, 1, fp); > + size = fread(assembly, *end, 1, fp); > fclose(fp); > - > + if (!size) { > + free(assembly); > + return NULL; > + } > return assembly; > } > > @@ -167,7 +173,11 @@ int main(int argc, char *argv[]) > > assembly = i965_disasm_read_binary(fp, ); > if (!assembly) { > - fprintf(stderr, "Unable to allocate buffer to read binary file\n"); > + if(end) > +fprintf(stderr, "Unable to allocate buffer to read binary > file\n"); > + else > +fprintf(stderr, "Input file is empty\n"); > + exit(EXIT_FAILURE); > } > > -- > 2.17.1 > > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v4 5/5] intel/tools: make sure the binary file is properly read
From: Andrii Simiklit 1. tools/i965_disasm.c:58:4: warning: ignoring return value of ‘fread’, declared with attribute warn_unused_result fread(assembly, *end, 1, fp); v2: Fixed incorrect return value check. ( Eric Engestrom ) v3: Zero size file check placed before fread with exit() ( Eric Engestrom ) v4: - Title is changed. - The 'size' variable was moved to top of a function scope. - The assertion was replaced by the proper error handling. - The error message on a caller side was fixed. ( Eric Engestrom ) Signed-off-by: Andrii Simiklit --- src/intel/tools/i965_disasm.c | 16 +--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/src/intel/tools/i965_disasm.c b/src/intel/tools/i965_disasm.c index 73a6760fc1..0efbdab706 100644 --- a/src/intel/tools/i965_disasm.c +++ b/src/intel/tools/i965_disasm.c @@ -47,17 +47,23 @@ i965_disasm_get_file_size(FILE *fp) static void * i965_disasm_read_binary(FILE *fp, size_t *end) { + size_t size; void *assembly; *end = i965_disasm_get_file_size(fp); + if (!*end) + return NULL; assembly = malloc(*end + 1); if (assembly == NULL) return NULL; - fread(assembly, *end, 1, fp); + size = fread(assembly, *end, 1, fp); fclose(fp); - + if (!size) { + free(assembly); + return NULL; + } return assembly; } @@ -167,7 +173,11 @@ int main(int argc, char *argv[]) assembly = i965_disasm_read_binary(fp, ); if (!assembly) { - fprintf(stderr, "Unable to allocate buffer to read binary file\n"); + if(end) +fprintf(stderr, "Unable to allocate buffer to read binary file\n"); + else +fprintf(stderr, "Input file is empty\n"); + exit(EXIT_FAILURE); } -- 2.17.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev