Skip to content

Fix TabReader error handling#255

Merged
MatthewThe merged 1 commit intopercolator:masterfrom
lkuchenb:fix/strtol
Apr 28, 2020
Merged

Fix TabReader error handling#255
MatthewThe merged 1 commit intopercolator:masterfrom
lkuchenb:fix/strtol

Conversation

@lkuchenb
Copy link
Contributor

There are no guarantees about errno when there is no error. I ran into a situation where errno==ENOMEM on our system (for unknown reasons, unfortunately) which definately didn't come from strtol().

It triggered the "your scan number is not an integer" error and percolator quit. This patch fixes the error handling for strtol() and we're not experiencing any problems with it anymore, however, I remain puzzled how errno was set to ENOMEM 🤷‍♂️

@MatthewThe
Copy link
Collaborator

Thanks for the pull request! This is indeed a corner case that we are currently not handling well.

We need to add errno=0 to the other reader.readX() functions as well. Furthermore, I think that the reason why we did not do this previously, was to make sure that even when a subsequent strtox() function worked fine, we would still raise an error the next time we check reader.error(). In this way, we can do multiple reader.readX() calls, but only have to check reader.error() once at the end.

To preserve this behavior we should only set err = errno if errno != 0. Unfortunately, this might sometimes give an error message that is not actually applicable to the actual problem (as you have experienced), but it will at least show us the approximate part of the code that causes the error.

@lkuchenb lkuchenb changed the title Fix strtol() error handling Fix TabReader error handling Apr 16, 2020
@lkuchenb
Copy link
Contributor Author

We need to add errno=0 to the other reader.readX() functions as well.

Indeed! I added it for strtod() and removed errno handling from the strchr() call since it doesn't even set errno.

Furthermore, I think that the reason why we did not do this previously, was to make sure that even when a subsequent strtox() function worked fine, we would still raise an error the next time we check reader.error(). In this way, we can do multiple reader.readX() calls, but only have to check reader.error() once at the end. To preserve this behavior we should only set err = errno if errno != 0

I added the conditional assignment, too.

@MatthewThe
Copy link
Collaborator

Thanks a lot! I will test this out after I finish with the other pull request.

@MatthewThe MatthewThe merged commit 749db8f into percolator:master Apr 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants