Identifying typos in a GTK 4 project using PVS-Studio

0793_GTK_4_continue_ru / image1.png







You may have already read a recent article about the process of starting PVS-Studio for the first time using the example of a GTK 4 project and about the primary filtering of warnings. Now it's time to work with the received report in more detail. And as our regular readers have already guessed, I bring to your attention an article describing the errors found in the code.







GTK 4 project has good code quality



I am not always in the mood to write more errors in the article, as was the case, for example, in the case of the recent publication " Espressif IoT Development Framework: 71 Shots in the Leg ". This time I will limit myself to 21 errors in honor of 2021 :). Plus, in fairness, it should be noted the high quality of the GTK 4 project, and that the density of real errors is very low.







, , , , 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







- . , , .







PVS-Studio , , . , . , .







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++, , , , . , , , .







, . , , , :







  1. CWE-14: Compiler Removal of Code to Clear Buffers


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.








All Articles