More const-marking cleanup

Поиск
Список
Период
Сортировка
От Tom Lane
Тема More const-marking cleanup
Дата
Msg-id 1324889.1764886170@sss.pgh.pa.us
обсуждение исходный текст
Ответы Re: More const-marking cleanup
Re: More const-marking cleanup
Список pgsql-hackers
I noticed that our two buildfarm animals that are running Fedora
rawhide (caiman, midge) are emitting warnings like

compression.c: In function "parse_compress_options":
compression.c:458:13: warning: assignment discards "const" qualifier from pointer target type [-Wdiscarded-qualifiers]
  458 |         sep = strchr(option, ':');
      |             ^

Apparently, latest gcc is able to notice that constructions like

    const char *str = ...;
    char       *ptr = strchr(str, ':');

are effectively casting away const.  This is a good thing and long
overdue, but we have some work to do to clean up the places where
we are doing that.

Attached is a patch that cleans up all the cases I saw on a local
rawhide installation.  Most of it is unremarkable, but the changes
in ecpg are less so.  In particular, variable.c is a mess, because
somebody const-ified some of the char* arguments to find_struct()
and find_struct_member() without regard for the fact that all their
char* arguments point at the same string.  (Not that you could
discover that from their nonexistent documentation.)  I considered
just reverting that change, but was able to fix things up reasonably
well at the price of a few extra strdup's.  It turned out that
find_struct_member()'s modification of its input string is actually
entirely useless, because it undoes that before consulting the
string again.  find_struct() and find_variable() need to make
copies, though.

            regards, tom lane

diff --git a/src/backend/catalog/pg_type.c b/src/backend/catalog/pg_type.c
index 257c7da8568..4a69a81b9fb 100644
--- a/src/backend/catalog/pg_type.c
+++ b/src/backend/catalog/pg_type.c
@@ -948,7 +948,7 @@ char *
 makeMultirangeTypeName(const char *rangeTypeName, Oid typeNamespace)
 {
     char       *buf;
-    char       *rangestr;
+    const char *rangestr;

     /*
      * If the range type name contains "range" then change that to
diff --git a/src/backend/tsearch/spell.c b/src/backend/tsearch/spell.c
index 146801885d7..0dab31f0ddb 100644
--- a/src/backend/tsearch/spell.c
+++ b/src/backend/tsearch/spell.c
@@ -2320,7 +2320,7 @@ CheckCompoundAffixes(CMPDAffix **ptr, const char *word, int len, bool CheckInPla
     }
     else
     {
-        char       *affbegin;
+        const char *affbegin;

         while ((*ptr)->affix)
         {
diff --git a/src/backend/utils/adt/formatting.c b/src/backend/utils/adt/formatting.c
index 5bfeda2ffde..c3cb022a400 100644
--- a/src/backend/utils/adt/formatting.c
+++ b/src/backend/utils/adt/formatting.c
@@ -1046,8 +1046,9 @@ typedef struct NUMProc
     char       *number,            /* string with number    */
                *number_p,        /* pointer to current number position */
                *inout,            /* in / out buffer    */
-               *inout_p,        /* pointer to current inout position */
-               *last_relevant,    /* last relevant number after decimal point */
+               *inout_p;        /* pointer to current inout position */
+
+    const char *last_relevant,    /* last relevant number after decimal point */

                *L_negative_sign,    /* Locale */
                *L_positive_sign,
@@ -1118,7 +1119,7 @@ static FormatNode *NUM_cache(int len, NUMDesc *Num, const text *pars_str, bool *
 static char *int_to_roman(int number);
 static int    roman_to_int(NUMProc *Np, size_t input_len);
 static void NUM_prepare_locale(NUMProc *Np);
-static char *get_last_relevant_decnum(const char *num);
+static const char *get_last_relevant_decnum(const char *num);
 static void NUM_numpart_from_char(NUMProc *Np, int id, size_t input_len);
 static void NUM_numpart_to_char(NUMProc *Np, int id);
 static char *NUM_processor(FormatNode *node, NUMDesc *Num, char *inout,
@@ -5297,10 +5298,10 @@ NUM_prepare_locale(NUMProc *Np)
  * If there is no decimal point, return NULL (which will result in same
  * behavior as if FM hadn't been specified).
  */
-static char *
+static const char *
 get_last_relevant_decnum(const char *num)
 {
-    char       *result,
+    const char *result,
                *p = strchr(num, '.');

 #ifdef DEBUG_TO_FROM_CHAR
diff --git a/src/backend/utils/adt/xid8funcs.c b/src/backend/utils/adt/xid8funcs.c
index a211a107767..4b3f7a69b3b 100644
--- a/src/backend/utils/adt/xid8funcs.c
+++ b/src/backend/utils/adt/xid8funcs.c
@@ -194,7 +194,7 @@ is_visible_fxid(FullTransactionId value, const pg_snapshot *snap)
 #ifdef USE_BSEARCH_IF_NXIP_GREATER
     else if (snap->nxip > USE_BSEARCH_IF_NXIP_GREATER)
     {
-        void       *res;
+        const void *res;

         res = bsearch(&value, snap->xip, snap->nxip, sizeof(FullTransactionId),
                       cmp_fxid);
diff --git a/src/bin/pg_waldump/pg_waldump.c b/src/bin/pg_waldump/pg_waldump.c
index c6d6ba79e44..1c1ccf59f65 100644
--- a/src/bin/pg_waldump/pg_waldump.c
+++ b/src/bin/pg_waldump/pg_waldump.c
@@ -160,7 +160,7 @@ create_fullpage_directory(char *path)
 static void
 split_path(const char *path, char **dir, char **fname)
 {
-    char       *sep;
+    const char *sep;

     /* split filepath into directory & filename */
     sep = strrchr(path, '/');
diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c
index 68774a59efd..00593fab5e1 100644
--- a/src/bin/pgbench/pgbench.c
+++ b/src/bin/pgbench/pgbench.c
@@ -6254,7 +6254,7 @@ findBuiltin(const char *name)
 static int
 parseScriptWeight(const char *option, char **script)
 {
-    char       *sep;
+    const char *sep;
     int            weight;

     if ((sep = strrchr(option, WSEP)))
diff --git a/src/common/compression.c b/src/common/compression.c
index 4c3c9fd7b50..eb434542fbd 100644
--- a/src/common/compression.c
+++ b/src/common/compression.c
@@ -425,7 +425,7 @@ validate_compress_specification(pg_compress_specification *spec)
 void
 parse_compress_options(const char *option, char **algorithm, char **detail)
 {
-    char       *sep;
+    const char *sep;
     char       *endp;
     long        result;

diff --git a/src/interfaces/ecpg/pgtypeslib/datetime.c b/src/interfaces/ecpg/pgtypeslib/datetime.c
index 1b253747fc4..f43343b4594 100644
--- a/src/interfaces/ecpg/pgtypeslib/datetime.c
+++ b/src/interfaces/ecpg/pgtypeslib/datetime.c
@@ -335,8 +335,8 @@ PGTYPESdate_defmt_asc(date * d, const char *fmt, const char *str)
      */
     int            token[3][2];
     int            token_values[3] = {-1, -1, -1};
-    char       *fmt_token_order;
-    char       *fmt_ystart,
+    const char *fmt_token_order;
+    const char *fmt_ystart,
                *fmt_mstart,
                *fmt_dstart;
     unsigned int i;
diff --git a/src/interfaces/ecpg/preproc/ecpg.trailer b/src/interfaces/ecpg/preproc/ecpg.trailer
index 6f94b832a03..be94ca5bbfa 100644
--- a/src/interfaces/ecpg/preproc/ecpg.trailer
+++ b/src/interfaces/ecpg/preproc/ecpg.trailer
@@ -1975,12 +1975,14 @@ civarind: cvariable indicator

 char_civar: char_variable
     {
-        char       *ptr = strstr(@1, ".arr");
+        char       *var = mm_strdup(@1);
+        char       *ptr = strstr(var, ".arr");

         if (ptr)                /* varchar, we need the struct name here, not
                                  * the struct element */
             *ptr = '\0';
-        add_variable_to_head(&argsinsert, find_variable(@1), &no_indicator);
+        add_variable_to_head(&argsinsert, find_variable(var), &no_indicator);
+        @$ = var;
     }
     ;

diff --git a/src/interfaces/ecpg/preproc/variable.c b/src/interfaces/ecpg/preproc/variable.c
index 2c67e33e92e..350e780c000 100644
--- a/src/interfaces/ecpg/preproc/variable.c
+++ b/src/interfaces/ecpg/preproc/variable.c
@@ -6,6 +6,19 @@

 static struct variable *allvariables = NULL;

+/* this probably belongs in util.c, but for now it's only needed here */
+static char *
+mm_nstrdup(const char *in, size_t len)
+{
+    char       *out;
+
+    out = mm_alloc(len + 1);
+    memcpy(out, in, len);
+    out[len] = '\0';
+
+    return out;
+}
+
 struct variable *
 new_variable(const char *name, struct ECPGtype *type, int brace_level)
 {
@@ -22,17 +35,11 @@ new_variable(const char *name, struct ECPGtype *type, int brace_level)
 }

 static struct variable *
-find_struct_member(const char *name, char *str, struct ECPGstruct_member *members, int brace_level)
+find_struct_member(const char *name, const char *str,
+                   struct ECPGstruct_member *members, int brace_level)
 {
-    char       *next = strpbrk(++str, ".-["),
-               *end,
-                c = '\0';
-
-    if (next != NULL)
-    {
-        c = *next;
-        *next = '\0';
-    }
+    const char *next = strpbrk(++str, ".-["),
+               *end;

     for (; members; members = members->next)
     {
@@ -54,8 +61,7 @@ find_struct_member(const char *name, char *str, struct ECPGstruct_member *member
             }
             else
             {
-                *next = c;
-                if (c == '[')
+                if (*next == '[')
                 {
                     int            count;

@@ -123,25 +129,24 @@ find_struct_member(const char *name, char *str, struct ECPGstruct_member *member
 }

 static struct variable *
-find_struct(const char *name, char *next, char *end)
+find_struct(const char *name, const char *next, const char *end)
 {
+    char       *prefix;
     struct variable *p;
-    char        c = *next;

     /* first get the mother structure entry */
-    *next = '\0';
-    p = find_variable(name);
+    prefix = mm_nstrdup(name, next - name);
+    p = find_variable(prefix);

-    if (c == '-')
+    if (*next == '-')
     {
         if (p->type->type != ECPGt_array)
-            mmfatal(PARSE_ERROR, "variable \"%s\" is not a pointer", name);
+            mmfatal(PARSE_ERROR, "variable \"%s\" is not a pointer", prefix);

         if (p->type->u.element->type != ECPGt_struct && p->type->u.element->type != ECPGt_union)
-            mmfatal(PARSE_ERROR, "variable \"%s\" is not a pointer to a structure or a union", name);
+            mmfatal(PARSE_ERROR, "variable \"%s\" is not a pointer to a structure or a union", prefix);

-        /* restore the name, we will need it later */
-        *next = c;
+        free(prefix);

         return find_struct_member(name, ++end, p->type->u.element->u.members, p->brace_level);
     }
@@ -150,23 +155,21 @@ find_struct(const char *name, char *next, char *end)
         if (next == end)
         {
             if (p->type->type != ECPGt_struct && p->type->type != ECPGt_union)
-                mmfatal(PARSE_ERROR, "variable \"%s\" is neither a structure nor a union", name);
+                mmfatal(PARSE_ERROR, "variable \"%s\" is neither a structure nor a union", prefix);

-            /* restore the name, we will need it later */
-            *next = c;
+            free(prefix);

             return find_struct_member(name, end, p->type->u.members, p->brace_level);
         }
         else
         {
             if (p->type->type != ECPGt_array)
-                mmfatal(PARSE_ERROR, "variable \"%s\" is not an array", name);
+                mmfatal(PARSE_ERROR, "variable \"%s\" is not an array", prefix);

             if (p->type->u.element->type != ECPGt_struct && p->type->u.element->type != ECPGt_union)
-                mmfatal(PARSE_ERROR, "variable \"%s\" is not a pointer to a structure or a union", name);
+                mmfatal(PARSE_ERROR, "variable \"%s\" is not a pointer to a structure or a union", prefix);

-            /* restore the name, we will need it later */
-            *next = c;
+            free(prefix);

             return find_struct_member(name, end, p->type->u.element->u.members, p->brace_level);
         }
@@ -192,7 +195,7 @@ find_simple(const char *name)
 struct variable *
 find_variable(const char *name)
 {
-    char       *next,
+    const char *next,
                *end;
     struct variable *p;
     int            count;
@@ -227,15 +230,14 @@ find_variable(const char *name)
                 p = find_struct(name, next, end);
             else
             {
-                char        c = *next;
+                char       *prefix = mm_nstrdup(name, next - name);

-                *next = '\0';
-                p = find_simple(name);
+                p = find_simple(prefix);
                 if (p == NULL)
-                    mmfatal(PARSE_ERROR, "variable \"%s\" is not declared", name);
+                    mmfatal(PARSE_ERROR, "variable \"%s\" is not declared", prefix);
                 if (p->type->type != ECPGt_array)
-                    mmfatal(PARSE_ERROR, "variable \"%s\" is not a pointer", name);
-                *next = c;
+                    mmfatal(PARSE_ERROR, "variable \"%s\" is not a pointer", prefix);
+                free(prefix);
                 switch (p->type->u.element->type)
                 {
                     case ECPGt_array:
diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c
index efc41fca2ba..58d41207605 100644
--- a/src/test/regress/pg_regress.c
+++ b/src/test/regress/pg_regress.c
@@ -693,7 +693,7 @@ static
 const char *
 get_expectfile(const char *testname, const char *file)
 {
-    char       *file_type;
+    const char *file_type;
     _resultmap *rm;

     /*
diff --git a/src/timezone/zic.c b/src/timezone/zic.c
index a8c1de9910d..8dcc7b337a7 100644
--- a/src/timezone/zic.c
+++ b/src/timezone/zic.c
@@ -2620,7 +2620,7 @@ doabbr(char *abbr, struct zone const *zp, char const *letters,
        bool isdst, zic_t save, bool doquotes)
 {
     char       *cp;
-    char       *slashp;
+    char const *slashp;
     size_t        len;
     char const *format = zp->z_format;


В списке pgsql-hackers по дате отправления: