Fix #36, handle read failure #37
Conversation
zanzaben
force-pushed
the
fix36_read_fail_case
branch
from
c3277d5
to
e90d4a8
Compare
| printf("\ncfe_ts_crc error: file read failed!\n"); | ||
| exit(1); | ||
| } | ||
| fileCRC = CalculateCRC(buffer, (uint32)readSize, fileCRC); |
There was a problem hiding this comment.
Is there a reason that a cast to (uint32) was added here? Shouldn't be needed.
Also see my other comment in the other PR that the return type of read() is ssize_t, not int.
There was a problem hiding this comment.
It was done because CalculateCRC is expecting a uint32
There was a problem hiding this comment.
But it will convert implicitly - and forcing a conversion can have unintended consequences. In reality the CalculateCRC() functions should take a size_t anyway.
Note that C has very specific rules about when and how implicit type conversions are done - and things (almost) always just work. When it doesn't just work you'll get a compiler warning about it, but a cast can hide that warning because it is literally telling the compiler "I meant to do that". Hence why its not advised to put them in unless there is a real reason why you need to tell the compiler that you meant to do that. In this case, I don't see any reason.
There was a problem hiding this comment.
Note - I'm not suggesting changing CalculateCRC to size_t here - just leave it alone - it doesn't matter. My only concern is the extra cast - should not do this.
|
Note - submitted issue #41 about the EOF bit. Maybe we can fix that too while looking at this code. |
|
Moving this to #42 |
Describe the contribution
Fixes #36
Handles case where read returns an error
Testing performed
Build and run unit test
Expected behavior changes
No impact to behavior
System(s) tested on
Ubuntu 20.04
Contributor Info - All information REQUIRED for consideration of pull request
Alex Campbell GSFC