Repository navigation
logger: Log fread error only when it happened - #3463
Conversation
5467b77 to
59b0054
Compare
59b0054 to
6973afd
Compare
marc-hb
left a comment
marc-hb
left a comment
There was a problem hiding this comment.
The while (!ferror(global_config->in_fd)) { still doesn't make sense to me. Should it not be while(!feof)? What is the expected code path when no error ever occurs and feof is met?
There was a problem hiding this comment.
Sorry I removed ferror() here in commit 337afc4, my bad. After doing my homework (better late than never) I think it's better to add ferror() back here and use it instead of !feof() because the standard doesn't state that feof and ferror are mutually exclusive. In fact you could imagine both of them being true.
https://pubs.opengroup.org/onlinepubs/9699919799/functions/fread.html
There was a problem hiding this comment.
BTW the if(!ret) is really confusing, please change it to if(ret<1) to make it consistent with the fread() specification.
|
337afc4: doesn't work in github, use 12 characters and no trailing colon: Please also run this command: git config --global core.abbrev 12 Only once per user account and done! It's required for the Linux kernel and is better for everything else. |
6973afd to
5e9be11
Compare
fread may return 0 when some error occurred or nothig has been reded. Moreover errno may not be set on Windows plaform, use ferror instead. See: https://www.gnu.org/software/gnulib/manual/html_node/fread.html Fixes: 337afc4: ("logger: add error message when reading sof/etrace instead of sof/trace") Signed-off-by: Karol Trzcinski <karolx.trzcinski@linux.intel.com>
log format text comes from ldc file (may be invalid), so error check is needed. Don't treat it as fatal error, because this error may not impact following logs, when formatting text is root cause of the issue. Signed-off-by: Karol Trzcinski <karolx.trzcinski@linux.intel.com>
Windows platform needs "rb" mode to read binary file, otherwise after read 0x1A EOF will be. See: https://stackoverflow.com/questions/11717120/unexpected-return-value-from-fread Signed-off-by: Karol Trzcinski <karolx.trzcinski@linux.intel.com>
fread() returns number of readed blocks, 0 when nothing read. Comparison fread return value with "!ret" is quite misleading - may suggests that negative value is returrned after fail. Swapping if content makes flow easier, then first is error check, and eventyally return statement, next try to reopen file. It allows to check error condition only in one place, so there won't be possibility to use different error checks in subsequent stages (like ferror() and errno). in_file alignment with trace entry size check has been added, to warn about corrupted file. Signed-off-by: Karol Trzcinski <karolx.trzcinski@linux.intel.com>
5e9be11 to
7a582c7
Compare
|
@ktrzcinx we will get this in v1.6 final. |
Uh oh!
There was an error while loading. Please reload this page.