Möglicherweise haben Sie bereits den kürzlich erschienenen Artikel über das erstmalige Starten von PVS-Studio am Beispiel des GTK 4-Projekts und über die primäre Filterung von Warnungen gelesen. Jetzt ist es Zeit, mit dem erhaltenen Bericht detaillierter zu arbeiten. Und wie unsere regelmäßigen Leser bereits erraten haben, mache ich Sie auf einen Artikel aufmerksam, in dem die im Code gefundenen Fehler beschrieben werden.
Das GTK 4-Projekt hat eine gute Codequalität
Ich bin nicht immer in der Stimmung, mehr Fehler in den Artikel zu schreiben, wie dies beispielsweise bei der jüngsten Veröffentlichung " Espressif IoT Development Framework: 71 Shots in the Foot " der Fall war . Dieses Mal werde ich mich auf 21 Fehler zu Ehren von 2021 beschränken :). Fairerweise sollte außerdem die hohe Qualität des GTK 4-Projekts beachtet werden und die Dichte der tatsächlichen Fehler ist sehr gering.
, , , , Clang static analysis tool, Coverity, AddressSanitizer, UndefinedBehavior Sanitizer. , , - — .
GTK 4 , "GTK: ". 581 ( ). 581 : ? .
, , , . - . , , . , , . .
, :
bool var;
var = true;
if (!var && foo)
, , . ? - var? , , . PVS-Studio "A part of conditional expression is always false: !var". ? .
, , . GTK 4, :
gboolean debug_enabled;
#ifdef G_ENABLE_DEBUG
debug_enabled = TRUE;
#else
debug_enabled = FALSE;
#endif
....
if (!debug_enabled && !keys[i].always_enabled)
: V560 [CWE-570] A part of conditional expression is always false: !debug_enabled. gdk.c 281
, . , . .
, . , . .
N1.
void
gsk_vulkan_image_upload_regions (GskVulkanImage *self,
GskVulkanUploader *uploader,
guint num_regions,
GskImageRegion *regions)
{
....
for (int i = 0; i < num_regions; i++)
{
m = mem + offset;
if (regions[i].stride == regions[i].width * 4)
{
memcpy (m, regions[i].data, regions[i].stride * regions[i].height);
}
else
{
for (gsize r = 0; r < regions[i].height; i++) // <=
memcpy (m + r * regions[i].width * 4,
regions[i].data + r * regions[i].stride, regions[i].width * 4);
}
....
}
....
}
PVS-Studio: V533 [CWE-691] It is likely that a wrong variable is being incremented inside the 'for' operator. Consider reviewing 'i'. gskvulkanimage.c 721
, r, i. - . !
N2.
. , memcpy - , . .. Segmentation fault.
, :
GtkCssValue *
_gtk_css_border_value_parse (GtkCssParser *parser,
GtkCssNumberParseFlags flags,
gboolean allow_auto,
gboolean allow_fill)
{
....
guint i;
....
for (; i < 4; i++) // <=
{
if (result->values[(i - 1) >> 1])
result->values[i] = _gtk_css_value_ref (result->values[(i - 1) >> 1]);
}
result->is_computed = TRUE;
for (; i < 4; i++) // <=
if (result->values[i] && !gtk_css_value_is_computed (result->values[i]))
{
result->is_computed = FALSE;
break;
}
....
}
PVS-Studio: V621 [CWE-835] Consider inspecting the 'for' operator. It's possible that the loop will be executed incorrectly or won't be executed at all. gtkcssbordervalue.c 221
, i 4. , . .
N3.
static void
gtk_list_base_class_init (GtkListBaseClass *klass)
{
....
properties[PROP_ORIENTATION] =
g_param_spec_enum ("orientation",
P_("Orientation"),
P_("The orientation of the orientable"),
GTK_TYPE_ORIENTATION,
GTK_ORIENTATION_VERTICAL,
G_PARAM_READWRITE | G_PARAM_EXPLICIT_NOTIFY |
G_PARAM_EXPLICIT_NOTIFY);
....
}
PVS-Studio: V501 There are identical sub-expressions 'G_PARAM_EXPLICIT_NOTIFY' to the left and to the right of the '|' operator. gtklistbase.c 1151
G_PARAM_EXPLICIT_NOTIFY. , .
N4.
post_insert_fixup. char_count_delta line_count_delta.
static void post_insert_fixup (GtkTextBTree *tree,
GtkTextLine *insert_line,
int char_count_delta,
int line_count_delta);
, , :
void
_gtk_text_btree_insert (GtkTextIter *iter,
const char *text,
int len)
{
....
int line_count_delta; /* Counts change to total number of
* lines in file.
*/
int char_count_delta; /* change to number of chars */
....
post_insert_fixup (tree, line, line_count_delta, char_count_delta);
....
}
PVS-Studio: V764 [CWE-683] Possible incorrect order of arguments passed to 'post_insert_fixup' function: 'line_count_delta' and 'char_count_delta'. gtktextbtree.c 1230
- . , , .
N5.
:
static guint
translate_keysym (GdkX11Keymap *keymap_x11,
guint hardware_keycode,
int group,
GdkModifierType state,
int *effective_group,
int *effective_level)
{
....
}
, :
static gboolean
gdk_x11_keymap_translate_keyboard_state (GdkKeymap *keymap,
guint hardware_keycode,
GdkModifierType state,
int group,
guint *keyval,
int *effective_group,
int *level,
GdkModifierType *consumed_modifiers)
{
....
tmp_keyval = translate_keysym (keymap_x11, hardware_keycode,
group, state,
level, effective_group); // <=
....
}
PVS-Studio: V764 [CWE-683] Possible incorrect order of arguments passed to 'translate_keysym' function: 'level' and 'effective_group'. gdkkeys-x11.c 1386
- . : level effective_group.
- PVS-Studio, :). ? , , ?
N6. (*)
gboolean
gtk_check_compact_table (...., int n_compose, ....)
{
....
guint16 *seq_index;
....
seq_index = bsearch (compose_buffer,
table->data,
table->n_index_size,
sizeof (guint16) * table->n_index_stride,
compare_seq_index);
if (!seq_index)
return FALSE;
if (seq_index && n_compose == 1)
return TRUE;
....
}
PVS-Studio: V560 [CWE-571] A part of conditional expression is always true: seq_index. gtkimcontextsimple.c 475
seq_index . . , , , , , . , , . :
if (!seq_index)
return FALSE;
if (*seq_index && n_compose == 1)
return TRUE;
N7-N9.
static void
gtk_message_dialog_init (GtkMessageDialog *dialog)
{
GtkMessageDialogPrivate *priv = ....;
....
priv->has_primary_markup = FALSE;
priv->has_secondary_text = FALSE;
priv->has_primary_markup = FALSE;
priv->has_secondary_text = FALSE;
....
}
PVS-Studio:
- V519 [CWE-563] The 'priv->has_primary_markup' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 262, 264. gtkmessagedialog.c 264
- V519 [CWE-563] The 'priv->has_secondary_text' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 263, 265. gtkmessagedialog.c 265
:
priv->has_primary_markup = FALSE; priv->has_secondary_text = FALSE;
, . , - . - .
:
- V519 [CWE-563] The 'self->state' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 2851, 2855. gdkevents.c 2855
- V519 [CWE-563] The 'display->width' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 2635, 2640. gtktextlayout.c 2640
N10.
static gboolean
on_flash_timeout (GtkInspectorWindow *iw)
{
iw->flash_count++;
gtk_highlight_overlay_set_color (GTK_HIGHLIGHT_OVERLAY (iw->flash_overlay),
&(GdkRGBA) {
0.0, 0.0, 1.0,
(iw && iw->flash_count % 2 == 0) ? 0.0 : 0.2
});
....
}
PVS-Studio: V595 [CWE-476] The 'iw' pointer was utilized before it was verified against nullptr. Check lines: 194, 199. inspect-button.c 194
iw , . , . :
(iw && iw->flash_count % 2 == 0)
, :
if (iw)
iw->flash_count++;
gtk_highlight_overlay_set_color (GTK_HIGHLIGHT_OVERLAY (iw->flash_overlay),
&(GdkRGBA) {
0.0, 0.0, 1.0,
(iw && iw->flash_count % 2 == 0) ? 0.0 : 0.2
});
, :). , :
GTK_HIGHLIGHT_OVERLAY (iw->flash_overlay)
, , , .
N11.
static void
cups_dispatch_watch_finalize (GSource *source)
{
GtkPrintCupsDispatchWatch *dispatch;
....
const char *username;
char hostname[HTTP_MAX_URI];
char *key;
httpGetHostname (dispatch->request->http, hostname, sizeof (hostname));
if (is_address_local (hostname))
strcpy (hostname, "localhost");
if (dispatch->backend->username != NULL) // <=
username = dispatch->backend->username; // <=
else
username = cupsUser ();
key = g_strconcat (username, "@", hostname, NULL);
GTK_NOTE (PRINTING,
g_print ("CUPS backend: removing stored password for %s\n", key));
g_hash_table_remove (dispatch->backend->auth, key); // <=
g_free (key);
if (dispatch->backend) // <=
dispatch->backend->authentication_lock = FALSE;
....
}
PVS-Studio: V595 [CWE-476] The 'dispatch->backend' pointer was utilized before it was verified against nullptr. Check lines: 1603, 1613. gtkprintbackendcups.c 1603
"" :). dispatch->backend (. , ). , - ! :
if (dispatch->backend)
:).
N12. ,
static GskRenderNode *
gtk_snapshot_collect_blend_top (GtkSnapshot *snapshot,
GtkSnapshotState *state,
GskRenderNode **nodes,
guint n_nodes)
{
GskRenderNode *bottom_node, *top_node, *blend_node;
GdkRGBA transparent = { 0, 0, 0, 0 };
top_node = gtk_snapshot_collect_default (snapshot, state, nodes, n_nodes);
bottom_node = state->data.blend.bottom_node != NULL
? gsk_render_node_ref (state->data.blend.bottom_node)
: NULL;
g_assert (top_node != NULL || bottom_node != NULL);
if (top_node == NULL)
top_node = gsk_color_node_new (&transparent, &bottom_node->bounds);
if (bottom_node == NULL)
bottom_node = gsk_color_node_new (&transparent, &top_node->bounds);
....
}
V595 [CWE-476] The 'bottom_node' pointer was utilized before it was verified against nullptr. Check lines: 1189, 1190. gtksnapshot.c 1189
, top_node bottom_node . :
g_assert (top_node != NULL || bottom_node != NULL);
, g_assert . . , :
if (top_node == NULL && bottom_node == NULL)
{
g_assert (false);
return NULL;
}
N13.
static void
stash_desktop_startup_notification_id (void)
{
const char *desktop_startup_id;
desktop_startup_id = g_getenv ("DESKTOP_STARTUP_ID");
if (desktop_startup_id && *desktop_startup_id != '\0')
{
if (!g_utf8_validate (desktop_startup_id, -1, NULL))
g_warning ("DESKTOP_STARTUP_ID contains invalid UTF-8");
else
startup_notification_id =
g_strdup (desktop_startup_id ? desktop_startup_id : "");
}
g_unsetenv ("DESKTOP_STARTUP_ID");
}
PVS-Studio: V547 [CWE-571] Expression 'desktop_startup_id' is always true. gdk.c 176
, , . , , -, , -, .
, :
desktop_startup_id = g_getenv ("DESKTOP_STARTUP_ID");
if (desktop_startup_id && *desktop_startup_id != '\0')
{
if (!g_utf8_validate (desktop_startup_id, -1, NULL))
g_warning ("DESKTOP_STARTUP_ID contains invalid UTF-8");
else
startup_notification_id = g_strdup (desktop_startup_id);
}
N14.
, . . / , . , , . , .
, :
#define MAX_LIST_SIZE 1000
static void
gtk_recent_manager_real_changed (GtkRecentManager *manager)
{
....
int age;
int max_size = MAX_LIST_SIZE;
....
.... // max_size .
....
if (age == 0 || max_size == 0 || !enabled)
{
g_bookmark_file_free (priv->recent_items);
priv->recent_items = g_bookmark_file_new ();
priv->size = 0;
}
else
{
if (age > 0)
gtk_recent_manager_clamp_to_age (manager, age);
if (max_size > 0)
gtk_recent_manager_clamp_to_size (manager, max_size);
}
....
}
PVS-Studio: V547 [CWE-571] Expression 'max_size > 0' is always true. gtkrecentmanager.c 480
, max_size , . . , , - .
. : "max_size == 0"? . . : V560 [CWE-570] A part of conditional expression is always false: max_size == 0. gtkrecentmanager.c 470.
N15, N16.
static void
action_handle_method (GtkAtSpiContext *self,
const char *method_name,
GVariant *parameters,
GDBusMethodInvocation *invocation,
const Action *actions,
int n_actions)
{
....
int idx = -1;
g_variant_get (parameters, "(i)", &idx);
const Action *action = &actions[idx];
if (idx >= 0 && idx < n_actions)
g_dbus_method_invocation_return_value (
invocation, g_variant_new ("(s)", action->name));
else
g_dbus_method_invocation_return_error (invocation,
G_IO_ERROR,
G_IO_ERROR_INVALID_ARGUMENT,
"Unknown action %d",
idx);
....
}
PVS-Studio: V781 [CWE-129] The value of the 'idx' variable is checked after it was used. Perhaps there is a mistake in program logic. Check lines: 71, 73. gtkatspiaction.c 71
idx :
const Action *action = &actions[idx];
, :
if (idx >= 0 && idx < n_actions)
, , , .
: V781 [CWE-129] The value of the 'idx' variable is checked after it was used. Perhaps there is a mistake in program logic. Check lines: 132, 134. gtkatspiaction.c 132
N17, N18.
static gboolean
parse_n_plus_b (GtkCssParser *parser,
int before,
int *a,
int *b)
{
const GtkCssToken *token;
token = gtk_css_parser_get_token (parser);
if (gtk_css_token_is_ident (token, "n"))
{
....
return parse_plus_b (parser, FALSE, b);
}
else if (gtk_css_token_is_ident (token, "n-"))
{
....
return parse_plus_b (parser, TRUE, b);
}
else if (gtk_css_token_is (token, GTK_CSS_TOKEN_IDENT) &&
string_has_number (token->string.string, "n-", b))
{
....
return TRUE;
}
else
{
*b = before;
*a = 0;
return TRUE;
}
gtk_css_parser_error_syntax (parser, "Not a valid an+b type");
return FALSE;
}
PVS-Studio: V779 [CWE-561] Unreachable code detected. It is possible that an error is present. gtkcssselector.c 1077
if-else-if-else… . , , .
: V779 [CWE-561] Unreachable code detected. It is possible that an error is present. gtktreemodelfilter.c 3289
N19, N20.
static void
gtk_paint_spinner (GtkStyleContext *context,
cairo_t *cr,
guint step,
int x,
int y,
int width,
int height)
{
GdkRGBA color;
guint num_steps;
double dx, dy;
....
dx = width / 2;
dy = height / 2;
....
cairo_move_to (cr,
dx + (radius - inset) * cos (i * G_PI / half),
dy + (radius - inset) * sin (i * G_PI / half));
cairo_line_to (cr,
dx + radius * cos (i * G_PI / half),
dy + radius * sin (i * G_PI / half));
....
}
PVS-Studio:
- V636 [CWE-682] The 'width / 2' expression was implicitly cast from 'int' type to 'double' type. Consider utilizing an explicit type cast to avoid the loss of a fractional part. An example: double A = (double)(X) / Y;. gtkcellrendererspinner.c 412
- V636 [CWE-682] The 'height / 2' expression was implicitly cast from 'int' type to 'double' type. Consider utilizing an explicit type cast to avoid the loss of a fractional part. An example: double A = (double)(X) / Y;. gtkcellrendererspinner.c 413
dx dy. , double. , :
dx = width / 2.0;
dy = height / 2.0;
, :
- V636 [CWE-682] The 'width / 2' expression was implicitly cast from 'int' type to 'double' type. Consider utilizing an explicit type cast to avoid the loss of a fractional part. An example: double A = (double)(X) / Y;. gtkswitch.c 255
- V636 [CWE-682] The 'width / 2' expression was implicitly cast from 'int' type to 'double' type. Consider utilizing an explicit type cast to avoid the loss of a fractional part. An example: double A = (double)(X) / Y;. gtkswitch.c 257
N21.
, . memset . , , , C C++, , , , . , , , .
, . , , , :
GTK 4? , free , .
, GTK 4 g_free. :
void
g_free (gpointer mem)
{
free (mem);
TRACE(GLIB_MEM_FREE((void*) mem));
}
g_free free? GLib 2.46 . :
It's important to match g_malloc() (and wrappers such as g_new()) with g_free(), g_slice_alloc() (and wrappers such as g_slice_new()) with g_slice_free(), plain malloc() with free(), and (if you're using C++) new with delete and new[] with delete[]. Otherwise bad things can happen, since these allocators may use different memory pools (and new/delete call constructors and destructors).
Since GLib 2.46 g_malloc() is hardcoded to always use the system malloc implementation.
, g_malloc malloc, free.
.
void overwrite_and_free (gpointer data)
{
char *password = (char *) data;
if (password != NULL)
{
memset (password, 0, strlen (password));
g_free (password);
}
}
PVS-Studio: V597 [CWE-14] The compiler could delete the 'memset' function call, which is used to flush 'password' object. The memset_s() function should be used to erase the private data. gtkprintbackendcups.c 848
, g_free. , , , . g_free overwrite_and_free, , memset .
.
PVS-Studio . -, IntelliJ IDEA, Rider, IncrediBuild, Jenkins, PlatformIO, Travis CI, GitLab CI/CD, CircleCI, TeamCity, Visual Studio . -, , , . , , , . , , , 10 , PVS-Studio Visual Studio. :). , .
, - . , - :). , , , SonarQube.
PVS-Studio. , , , , . , , . PVS-Studio , :). , , , ! : PVS-Studio (YouTube).
, : Andrey Karpov. Finding Typos in the GTK 4 Project by PVS-Studio.