Skip to content

gtk.cfg: Remove pure annotation from g_str_has_prefix/suffix - #8788

Open
correctmost wants to merge 1 commit into
cppcheck-opensource:mainfrom
correctmost:remove-pure-annotations
Open

gtk.cfg: Remove pure annotation from g_str_has_prefix/suffix#8788
correctmost wants to merge 1 commit into
cppcheck-opensource:mainfrom
correctmost:remove-pure-annotations

Conversation

@correctmost

Copy link
Copy Markdown
Contributor

g_str_has_prefix and g_str_has_suffix are not technically pure because they can log to the console if a precondition fails.

This partially reverts commit 7cc7c0b.


Some notes:

The original gtk.cfg change hasn't been released yet, so the revert shouldn't cause any churn for users.

GLib has code like this:

gboolean (g_str_has_prefix) (const gchar *str,
                             const gchar *prefix)
{
  g_return_val_if_fail (str != NULL, FALSE);
  g_return_val_if_fail (prefix != NULL, FALSE);

  return strncmp (str, prefix, strlen (prefix)) == 0;
}

Most real-world code is treating this function as safe to call inside of an assert, even though it could technically have side effects if one of the preconditions fails.

I removed the pure annotation to err on the side of correctness and pedantry, even though most projects would view the warnings as "false positives" from a practical perspective.

Here is example usage from QEMU:

char *qemu_chr_get_filename(Chardev *chr)
{
    ChardevClass *cc = CHARDEV_GET_CLASS(chr);
    const char *typename;

    if (cc->chr_get_filename) {
        return cc->chr_get_filename(chr);
    }

    typename = object_get_typename(OBJECT(chr));
    assert(g_str_has_prefix(typename, "chardev-"));
    return g_strdup(typename + 8);
}

g_str_has_prefix and g_str_has_suffix are not technically pure
because they can log to the console if a precondition fails.

This partially reverts commit 7cc7c0b.
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.

1 participant