db_tutorial icon indicating copy to clipboard operation
db_tutorial copied to clipboard

Part 4 regression: invalid integers parsed as 0

Open nilesr opened this issue 6 years ago • 0 comments

For the first few parts, when we're reading input to insert statements using scanf, scanf will return an error if the first field is not an integer. However in part 4 we stop using scanf and switch to strtok. To convert the ID field to an integer, we call atoi.

The problem is that if atoi is given a garbage string like "abcdef", it will just return zero, which is a valid ID. With scanf this would have correctly returned an error.

Additionally, the error messages surrounding IDs are further misleading. One of the tests we write tries to insert with an ID of negative one, and we fix the code to reject IDs that are less than zero. (if (id < 0) {). However the error message we print says "ID must be positive." This is misleading because 0 is a valid ID in the code, but 0 is not positive.

To fix this, we could replace atoi with strtol and check the endptr, and change the error message to "IDs may not be negative." However this would make the code more complex, so I wanted to ask about it in an issue before opening a pull request.

nilesr avatar Apr 07 '19 15:04 nilesr