Welcome to Software Development on Codidact!

Will you help us build our independent community of developers helping developers? We're small and trying to grow. We welcome questions about all aspects of software development, from design to code to QA and more. Got questions? Got answers? Got code you'd like someone to review? Please join us.

Post History

60%

+1 −0

Code Reviews A simple implementation of a mutable String in C

(User JS1 answered as follows; source.) Unsafe loop This loop in string_find() is unsafe since it could read past the end of your buffer: while (str->data[pos] != c) ++pos; You shoul...

posted 8d ago by aura-lsprog-86‭ · edited 8d ago by aura-lsprog-86‭

Answer

#2: Post edited by

aura-lsprog-86‭ · 2025-02-14T02:29:27Z (8 days ago)
Amend lack of MathJax in answer

Copy Link

Raw

Markdown

_(User [JS1](https://codereview.stackexchange.com/users/58193/js1) answered as follows; [source](https://codereview.stackexchange.com/a/156351/98306).)_
<hr />
## Unsafe loop
This loop in `string_find()` is unsafe since it could read past the end of your buffer:
> while (str->data[pos] != c) ++pos;
You should add an additional check like this:
while (pos < str->length && str->data[pos] != c) ++pos;
## Short circuit error conditions
Rather than doing this:
> if (str != NULL) {
> // Rest of function indented
> }
it would be easier to read if you rewrote it like this:
if (str == NULL)
return NULL;
// Rest of function, not indented
## Other things
~~Your reallocation strategy will lead to $O(n^2)$ behavior when appending to long strings. You might want to double the allocation instead of adding a fixed amount.~~
You might want to use `size_t` for your sizes and lengths instead if `int`, because an `int` might overflow at 32KB on some platforms.

_(User [JS1](https://codereview.stackexchange.com/users/58193/js1) answered as follows; [source](https://codereview.stackexchange.com/a/156351/98306).)_
<hr />
## Unsafe loop
This loop in `string_find()` is unsafe since it could read past the end of your buffer:
> while (str->data[pos] != c) ++pos;
You should add an additional check like this:
while (pos < str->length && str->data[pos] != c) ++pos;
## Short circuit error conditions
Rather than doing this:
> if (str != NULL) {
> // Rest of function indented
> }
it would be easier to read if you rewrote it like this:
if (str == NULL)
return NULL;
// Rest of function, not indented
## Other things
Your reallocation strategy will lead to **O(n^2)** behavior when appending to long strings. You might want to double the allocation instead of adding a fixed amount.
You might want to use `size_t` for your sizes and lengths instead if `int`, because an `int` might overflow at 32KB on some platforms.

#1: Initial revision by

aura-lsprog-86‭ · 2025-02-14T02:27:19Z (8 days ago)

Copy Link

Raw

Markdown

_(User [JS1](https://codereview.stackexchange.com/users/58193/js1) answered as follows; [source](https://codereview.stackexchange.com/a/156351/98306).)_

<hr />

## Unsafe loop

This loop in `string_find()` is unsafe since it could read past the end of your buffer:

>     while (str->data[pos] != c) ++pos;

You should add an additional check like this:

    while (pos < str->length && str->data[pos] != c) ++pos;

## Short circuit error conditions

Rather than doing this:

>     if (str != NULL) {
>         // Rest of function indented
>     }

it would be easier to read if you rewrote it like this:

    if (str == NULL)
        return NULL;

    // Rest of function, not indented

## Other things

Your reallocation strategy will lead to $O(n^2)$ behavior when appending to long strings. You might want to double the allocation instead of adding a fixed amount.

You might want to use `size_t` for your sizes and lengths instead if `int`, because an `int` might overflow at 32KB on some platforms.

Communities

Post History