Re: [PATCH v2] fixdep: add fstat error handling

[Ron Yorston]

Sam James  wrote:
>David Leonard  writes:
>> I worry that the fprintf() may destroy the errno which perror() uses,
>> so you could get a random error message.
>> Perhaps remove the fprintf(s) completely? Because the context should be
>> clear enough from the filename alone that perror displays.
>
>Ah, a great point. Any preference between just stripping the fprintfs vs
>a better argument to perror, as we do in some places (but not very
>consistently)?

Or:

   fprintf(stderr, "fixdep: fstat %s %s\n", depfile, strerror(errno));

Cheers,

Ron
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH v2] fixdep: add fstat error handling

[Sam James]

David Leonard  writes:

> On Tue, 23 Apr 2024, Sam James wrote:
>
>>  }
>> -fstat(fd, );
>> +if (fstat(fd, ) < 0) {
>> +fprintf(stderr, "fixdep: fstat");
>> +perror(filename);
>> +exit(2);
>> +}
>>  if (st.st_size == 0) {
>>  close(fd);
>>  return;
>> @@ -368,7 +372,11 @@ void print_deps(void)
>>  perror(depfile);
>>  exit(2);
>>  }
>> -fstat(fd, );
>> +if (fstat(fd, ) < 0) {
>> +fprintf(stderr, "fixdep: fstat");
>> +perror(depfile);
>> +exit(2);
>> +}
>>  if (st.st_size == 0) {
>>  fprintf(stderr,"fixdep: %s is empty\n",depfile);
>>  close(fd);
>
> I worry that the fprintf() may destroy the errno which perror() uses,
> so you could get a random error message.
> Perhaps remove the fprintf(s) completely? Because the context should be
> clear enough from the filename alone that perror displays.

Ah, a great point. Any preference between just stripping the fprintfs vs
a better argument to perror, as we do in some places (but not very
consistently)?

I don't think I have a strong preference.

>
> David

thanks,
sam
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH v2] fixdep: add fstat error handling

[David Leonard]

On Tue, 23 Apr 2024, Sam James wrote:


}
-   fstat(fd, );
+   if (fstat(fd, ) < 0) {
+   fprintf(stderr, "fixdep: fstat");
+   perror(filename);
+   exit(2);
+   }
if (st.st_size == 0) {
close(fd);
return;
@@ -368,7 +372,11 @@ void print_deps(void)
perror(depfile);
exit(2);
}
-   fstat(fd, );
+   if (fstat(fd, ) < 0) {
+   fprintf(stderr, "fixdep: fstat");
+   perror(depfile);
+   exit(2);
+   }
if (st.st_size == 0) {
fprintf(stderr,"fixdep: %s is empty\n",depfile);
close(fd);


I worry that the fprintf() may destroy the errno which perror() uses,
so you could get a random error message.
Perhaps remove the fprintf(s) completely? Because the context should be
clear enough from the filename alone that perror displays.

David
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox