3 ms·
The bug is that it's missing abstractions. 1) reading directly from a FILE. 2) Saving local state through a pointer (ptr). 3) weird way to report back the out s
by jstimpfle 3y ago
The bug is that it's missing abstractions. 1) reading directly from a FILE. 2) Saving local state through a pointer (ptr). 3) weird way to report back the out state, in case of EOF can't distinguish between ferror or not.
4) Probably it would be better to not check for errors right away -- just share a common path with EOF everywhere, and check for I/O errors only at the very end.
5) What is the use of this function? Not sure what's the point, it seems unergonomic. Probably better just code one loop for e.g. names ([_a-z][_a-z0-9]*) and integers / floats etc.
6) What's with the strange (*--ptr), overwriting previous work. Another bug waiting to happen.
7) Why are we returning out + skipped? Returning the consumed characters? That could just be coded in the parser input abstraction (where you would also keep track of the current file offset / possibly line and column (but you can compute those when you need them)).
8) What about the ferror(src) ? (skipped - out) : (out - skipped)?
9) 3 locations where a zero-terminator is written, when there should be _at most_ 1 location, but probably none.
If the code wouldn't do 2), the bug you describe would probably never have happened. The return value from fgetc() is int, not char. It has to be larger than char to be able to return a char as well as EOF.
I would probably code something along the lines of
void identifier(Parser *parser)
{
My_String_Builder *builder = get_string_builder(parser);
reset_string_builder(builder);
for (;;)
{
string_builder_push(builder, c);
if (! next_byte(parser))
break;
int c = parser_get(parser);
if (!(is_alpha(c) || c == '_' || ...))
break;
}
String *string = string_builder_finalize(builder);
// check string builder "overflow" / too long / further input sanitization
push_string_token(parser, TOKEN_IDENTIFIER, string);
}