[CBRD-25212] remove warning messages from compilation #4933

beyondykk9 · 2024-02-02T05:46:42Z

http://jira.cubrid.org/browse/CBRD-25212

We need remove the warning messages from compilation (CUBRID engine build). By doing this, we can prevent potential problems in advance.

The target source code is as follows.

src/base/
src/compat/
src/connection/
src/communication/
src/heaplayers/
src/thread/
src/object/
src/session/
src/jsp/
src/method/
src/parser/
src/query/
src/xasl/

ctshim · 2024-02-05T01:44:40Z

src/base/ddl_log.c

@@ -622,7 +622,7 @@ logddl_make_copy_filename (T_APP_NAME app_name, const char *file_full_path, char
  const char *name_tmp = NULL;
  int retval = 0;

-  if (file_full_path == NULL || copy_filename == NULL || buf_size < 0)
+  if (file_full_path == NULL || copy_filename == NULL)


I think it would be better to remove the buf_size argument and specify PATH_MAX directly.

I think the buf_size parameter should be kept. The reason is that the size of the string can change.

ctshim · 2024-02-05T01:44:47Z

src/base/ddl_log.c

@@ -658,7 +658,7 @@ logddl_make_copy_dir (T_APP_NAME app_name, char *copy_filename, char *copy_fullp
  const char *env_root = NULL;
  int retval = 0;

-  if (copy_filename == NULL || copy_fullpath == NULL || buf_size < 0)
+  if (copy_filename == NULL || copy_fullpath == NULL)


I think it would be better to remove the buf_size argument and specify PATH_MAX directly.

I think the buf_size parameter should be kept. The reason is that the size of the string can change.

src/parser/view_transform.c

src/query/execute_schema.c

src/parser/type_checking.c

src/query/execute_schema.c

airnet73 · 2024-02-07T07:31:05Z

src/executables/csql.c

@@ -162,7 +162,7 @@ char csql_Scratch_text[SCRATCH_TEXT_LEN];
 int csql_Error_code = NO_ERROR;

 static char csql_Prompt[100];
-static char csql_Prompt_offline[100];
+static char csql_Prompt_offline[101];


I think "snprintf (csql_Prompt_offline, sizeof (csql_Prompt_offline) - 1, "!%s", csql_Prompt);" is better than "csql_Prompt_offline[101]". (line 2772)

hyunikn · 2024-02-15T05:34:46Z

src/base/object_representation.h

@@ -2684,6 +2684,8 @@ or_get_oid (OR_BUF * buf, OID * oid)
 {
  ASSERT_ALIGN (buf->ptr, INT_ALIGNMENT);

+  OID_SET_NULL (oid);


This should be moved to the then part of the if statement below because setting the oid fields is done in else part.

Ok, I aggree.

hyunikn · 2024-02-15T05:52:53Z

src/object/object_domain.c

@@ -11518,7 +11518,7 @@ tp_value_auto_cast_with_precision_check (const DB_VALUE * src, DB_VALUE * dest,
      /* if the numeric's precision is 19 or more, then it can get the bigint enough */
      if (desired_domain->type->id == DB_TYPE_NUMERIC && desired_domain->precision < 19)
 	{
-	  INT64 bigint;
+	  INT64 bigint = 0;


This should be moved to the default branch of the switch statement below because this is overwritten in other branches.

hyunikn · 2024-02-15T05:56:24Z

src/object/object_primitive.c

@@ -3568,7 +3568,7 @@ static int
 mr_data_readval_utime (OR_BUF * buf, DB_VALUE * value, TP_DOMAIN * domain, int size, bool copy, char *copy_buf,
 		       int copy_buf_len)
 {
-  DB_UTIME utm;
+  DB_UTIME utm = 0;


This definition should be moved to the else branch of the if statement below because utm is used only in that branch.

hyunikn · 2024-02-15T05:56:52Z

src/object/object_primitive.c

@@ -3591,7 +3591,7 @@ static int
 mr_data_readval_timestampltz (OR_BUF * buf, DB_VALUE * value, TP_DOMAIN * domain, int size, bool copy, char *copy_buf,
 			      int copy_buf_len)
 {
-  DB_UTIME utm;
+  DB_UTIME utm = 0;


This definition should be moved to the else branch of the if statement below because utm is used only in that branch.

hyunikn · 2024-02-15T06:16:10Z

src/parser/parser_support.c

@@ -11607,7 +11607,7 @@ pt_convert_dblink_dml_query (PARSER_CONTEXT * parser, PT_NODE * node,
  PT_NODE *list = NULL;		/* for insert select list */
  PT_NODE *spec, *into_spec = NULL, *upd_spec = NULL, *server;

-  PARSER_VARCHAR *comment, *dml;
+  PARSER_VARCHAR *comment = NULL, *dml;


This NULL assignment should be moved to the default branch of the switch statement at line 11726 because in other branches the assignment is overwritten.

hyunikn · 2024-02-15T06:26:28Z

src/query/execute_statement.c

@@ -15348,12 +15348,12 @@ do_supplemental_statement (PARSER_CONTEXT * parser, PT_NODE * statement, RESERVE

 	    if (cls_info[i]->objtype == CDC_TABLE)
 	      {
-		strncpy (drop_stmt, drop_prefix, strlen (drop_prefix));
+		strncpy (drop_stmt, drop_prefix, strlen (drop_prefix) + 1);


strncpy(dst, src, strlen(src) + 1) is equivalent to strcpy(dst, src) and can be simplified to the latter.

I habitually use strncpy inside this function, but it would be better to replace it with strcpy.
If we go a little further,
A more desirable solution is possible by changing the order of the process of finding the length after copying.

strncpy (drop_stmt, drop_prefix, strlen (drop_prefix) + 1); drop_copied_length = strlen (drop_prefix);

Replace the form above as below (there is a small gain in performance)

drop_copied_length = strlen (drop_prefix); memcpy (drop_stmt, drop_prefix, len + 1);

ctshim · 2024-02-16T00:39:04Z

src/base/ini_parser.c

@@ -515,7 +515,7 @@ ini_parse_line (char *input_line, char *section, char *key, char *value)
      leading_char = section[0];
      if (leading_char == '@' || leading_char == '%')
 	{
-	  sprintf (section, "%c%s", leading_char, ini_str_trim (section + 1));
+	  snprintf (section, INI_BUFSIZ + 2, "%c%s", leading_char, ini_str_trim (section + 1));


The size of the section is (INI_BUFSIZ + 1).

ctshim · 2024-02-16T00:52:11Z

src/executables/compactdb_common.c

@@ -255,7 +255,7 @@ get_class_mops_from_file (const char *input_filename, MOP ** class_list, int *nu
 	  goto end;
 	}

-      strncpy (class_names[i], buffer, len);
+      strncpy (class_names[i], buffer, len + 1);


Is there a reason to copy with a size of "len+1"?
If you decide to modify it anyway, I recommend doing it as follows.

memcpy (class_names[i], buffer, len);

+1 is necessary because class_names[i] must end with a terminating zero. Isn't it?
Note that len is strlen(buffer).

@hyunikn
In the code, '\0' is filled with the substitution statement immediately below.
class_names[i][len] = 0;

ctshim · 2024-02-16T00:58:19Z

src/method/method_struct_query.cpp

@@ -493,7 +493,7 @@ namespace cubmethod
    fprintf (stdout, "tuple_count: %d\n", tuple_count);
    fprintf (stdout, "ins_oid (%d, %d, %d)\n", ins_oid.pageid, ins_oid.slotid, ins_oid.volid);
    fprintf (stdout, "include_oid: %d\n", include_oid);
-    fprintf (stdout, "query_id: %lld\n", query_id);
+    fprintf (stdout, "query_id: %ud\n", (unsigned int)query_id);


query_id is of type UINTPTR.
This is defined differently depending on define.
Therefore, if you do it like below, you will be able to see the warning again even if the type changes in the future.

fprintf (stdout, "query_id: %ud\n", query_id);

It must be %u rather than %ud.
Note that the following code prints '123d'

#include <stdio.h> int main() { printf("%ud\n", 123); }

ctshim · 2024-02-16T01:30:33Z

src/storage/btree_load.c

@@ -1149,8 +1148,9 @@ xbtree_load_index (THREAD_ENTRY * thread_p, BTID * btid, const char *bt_name, TP
      pr_clear_value (&load_args->current_key);

      BTID_SET_NULL (btid);
-      if (xbtree_add_index (thread_p, btid, key_type, &class_oids[0], attr_ids[0], unique_pk, sort_args->n_oids,
-			    sort_args->n_nulls, load_args->n_keys, btid_int.deduplicate_key_idx) == NULL)
+      if (xbtree_add_index (thread_p, btid, key_type, &class_oids[0], attr_ids[0], unique_pk, (INT64) sort_args->n_oids,


Maybe I should change the member variable of sort_args from int to INT64?

[CBRD-25212] remove warning messages from compilation

b65eb96

beyondykk9 added the refactoring label Feb 2, 2024

beyondykk9 added this to the fig-cake milestone Feb 2, 2024

beyondykk9 self-assigned this Feb 2, 2024

beyondykk9 requested review from hornetmj and shparkcubrid as code owners February 2, 2024 05:46

beyondykk9 added 2 commits February 2, 2024 14:50

revert unloaddb.h

2745f21

slip

6cacf3c

beyondykk9 requested review from hgryoo, hyunikn, ctshim and airnet73 February 2, 2024 07:11

ctshim reviewed Feb 5, 2024

View reviewed changes

src/parser/view_transform.c Outdated Show resolved Hide resolved

ctshim reviewed Feb 5, 2024

View reviewed changes

src/query/execute_schema.c Outdated Show resolved Hide resolved

ctshim reviewed Feb 5, 2024

View reviewed changes

src/parser/type_checking.c Outdated Show resolved Hide resolved

ctshim reviewed Feb 5, 2024

View reviewed changes

src/query/execute_schema.c Outdated Show resolved Hide resolved

beyondykk9 added 2 commits February 5, 2024 13:34

[[fallthrough]]

7e391be

[[fallthrough]]

2bdf722

shparkcubrid approved these changes Feb 7, 2024

View reviewed changes

airnet73 reviewed Feb 7, 2024

View reviewed changes

airnet73 approved these changes Feb 7, 2024

View reviewed changes

beyondykk9 added 4 commits February 8, 2024 17:29

fix warning for release mode

05982bf

inttypes.h

06fadd9

modified inttypes.h

72f8668

add __STDC_FORMAT_MACROS

d45c647

hyunikn reviewed Feb 15, 2024

View reviewed changes

ctshim reviewed Feb 16, 2024

View reviewed changes

hornetmj approved these changes Feb 20, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[CBRD-25212] remove warning messages from compilation #4933

[CBRD-25212] remove warning messages from compilation #4933

beyondykk9 commented Feb 2, 2024

ctshim Feb 5, 2024

airnet73 Feb 6, 2024

ctshim Feb 5, 2024

airnet73 Feb 6, 2024

airnet73 Feb 7, 2024 •

edited

Loading

hyunikn Feb 15, 2024

beyondykk9 Feb 16, 2024

hyunikn Feb 15, 2024

hyunikn Feb 15, 2024

hyunikn Feb 15, 2024

hyunikn Feb 15, 2024

hyunikn Feb 15, 2024 •

edited

Loading

ctshim Feb 16, 2024

ctshim Feb 16, 2024

ctshim Feb 16, 2024

hyunikn Feb 16, 2024

ctshim Feb 19, 2024

ctshim Feb 16, 2024

hyunikn Feb 16, 2024

ctshim Feb 16, 2024

[CBRD-25212] remove warning messages from compilation #4933

Are you sure you want to change the base?

[CBRD-25212] remove warning messages from compilation #4933

Conversation

beyondykk9 commented Feb 2, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

airnet73 Feb 7, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hyunikn Feb 15, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

airnet73 Feb 7, 2024 •

edited

Loading

hyunikn Feb 15, 2024 •

edited

Loading