4 ms·

Can someone explain the bugs in the code samples? The author says they're there but I honestly can't find them.

by jpochtar 7y ago

asib 7y ago
No checks regarding length of source and query - may end up dereferencing beyond the bounds of the string.
- ericlippert 7y ago
  There is a dereference past the bounds of the query in one case in the last code sample, but there is no deference beyond the bounds of the source string. You're probably thinking in C# or Java; remember that in C the convention is that a zero char ends strings. If the source string is shorter than the query string then the code will encounter a zero char in the source string at the same time as it encounters a non-zero char in the query string, and the inequality will end the loop before the beyond-bounds dereference. There are other defects; can you find them?
  - asib 7y ago
    No null checks in find. If source is null, dereferencing source[i] in the while condition will cause undefined behaviour. If just query is null, the same will occur in the first call to starts.
- jpochtar 7y ago
  There are though, when it checks for '\0'. `starts()` looks like it's not checking if len(source) < len(query), but if, say, query="foobar" and source="foo", when i = 3 the line if (source[i] != query[i]) return false; will evaluate to if ('\0' != 'b') return false; so `starts()` will correctly return false. Always if len(source) < len(query), we'll return false when we get to i=len(source), because source[len(source)] != query[len(source)] as source[len(source)] == '\0' and query[len(source)] != '\0' since len(query) > len(source).
ericlippert 7y ago
Hint: what is the correct behaviour of this method when given empty strings? Every string contains the empty string as a substring.
- deleted 7y ago
  [deleted]
- jpochtar 7y ago
  Oh, I'd assumed disagreement on behavior of query="" between the two code samples meant it was UB and was looking for crashes/invalid memory accesses.
  - ericlippert 7y ago
    A Visual Basic program is not allowed to have undefined behaviours like a C program; InStr has a specification and that specification has to be implemented; that spec includes defining the behaviour for all inputs. There's also no null handling here, which was a deliberate omission for clarity. In practice, the convention used inside the VB source code is that null string pointers are semantically the same as empty strings, which introduces some complexities.
    - jpochtar 7y ago
      ah neat, thanks!
- deleted 7y ago
  [deleted]
derangedHorse 7y ago
I don't see any checks preventing an 'out of bounds' exception nor general null checks before accessing data Edit: As others have stated, the 'out of bounds' exception should be taken care of by the '\0' at the end of strings in C

Can someone explain the bugs in the code samples? The author says they're there but I honestly can't find them.

asib 7y ago

No checks regarding length of source and query - may end up dereferencing beyond the bounds of the string.

ericlippert 7y ago
There is a dereference past the bounds of the query in one case in the last code sample, but there is no deference beyond the bounds of the source string. You're probably thinking in C# or Java; remember that in C the convention is that a zero char ends strings. If the source string is shorter than the query string then the code will encounter a zero char in the source string at the same time as it encounters a non-zero char in the query string, and the inequality will end the loop before the beyond-bounds dereference. There are other defects; can you find them?
- asib 7y ago
  No null checks in find. If source is null, dereferencing source[i] in the while condition will cause undefined behaviour. If just query is null, the same will occur in the first call to starts.
jpochtar 7y ago
There are though, when it checks for '\0'. `starts()` looks like it's not checking if len(source) < len(query), but if, say, query="foobar" and source="foo", when i = 3 the line if (source[i] != query[i]) return false; will evaluate to if ('\0' != 'b') return false; so `starts()` will correctly return false. Always if len(source) < len(query), we'll return false when we get to i=len(source), because source[len(source)] != query[len(source)] as source[len(source)] == '\0' and query[len(source)] != '\0' since len(query) > len(source).

ericlippert 7y ago

Hint: what is the correct behaviour of this method when given empty strings? Every string contains the empty string as a substring.

deleted 7y ago
[deleted]
jpochtar 7y ago
Oh, I'd assumed disagreement on behavior of query="" between the two code samples meant it was UB and was looking for crashes/invalid memory accesses.
- ericlippert 7y ago
  A Visual Basic program is not allowed to have undefined behaviours like a C program; InStr has a specification and that specification has to be implemented; that spec includes defining the behaviour for all inputs. There's also no null handling here, which was a deliberate omission for clarity. In practice, the convention used inside the VB source code is that null string pointers are semantically the same as empty strings, which introduces some complexities.
  - jpochtar 7y ago
    ah neat, thanks!
deleted 7y ago
[deleted]

derangedHorse 7y ago

I don't see any checks preventing an 'out of bounds' exception nor general null checks before accessing data Edit: As others have stated, the 'out of bounds' exception should be taken care of by the '\0' at the end of strings in C