Hi Ranier,

Following the comment in commit 9fd45870c1436b477264c0c82eb195df52bc0919,

    (The same could be done with appropriate memset() calls, but this
    patch is part of an effort to phase out MemSet(), so it doesn't touch
    memset() calls.)

Should these obviously possible replacement of the standard library function "memset" be considered as well? For example, something like the attached one which is focusing on the pageinspect extension only.


Best regards,

David

On 2022-08-01 10:08 a.m., Ranier Vilela wrote:
Em sáb., 16 de jul. de 2022 às 16:54, Ranier Vilela <ranier...@gmail.com> escreveu:



    Em sáb, 16 de jul de 2022 2:58 AM, Peter Eisentraut
    <peter.eisentr...@enterprisedb.com> escreveu:

        On 11.07.22 21:06, Ranier Vilela wrote:
        > Em qui., 7 de jul. de 2022 às 14:01, Ranier Vilela
        <ranier...@gmail.com
        > <mailto:ranier...@gmail.com>> escreveu:
        >
        >     Attached the v1 of your patch.
        >     I think that all is safe to switch MemSet by {0}.
        >
        > Here the rebased patch v2, against latest head.

        I have committed my patch with Álvaro's comments addressed

    I see.
    It's annoing that old compiler (gcc 4.7.2) don't handle this style.


        Your patch appears to add in changes that are either arguably
        out of
        scope or would need further review (e.g., changing memset()
        calls,
        changing the scope of some variables, changing places that
        need to worry
        about padding bits).  Please submit separate patches for
        those, and we
        can continue the analysis.

    Sure.

Hi, sorry for the delay.
Like how https://github.com/postgres/postgres/commit/9fd45870c1436b477264c0c82eb195df52bc0919
New attempt to remove more MemSet calls, that are safe.

Attached v3 patch.

regards,
Ranier Vilela
From 25bd8af7acfd6bf2e23cdfac93828d810cfbb5b4 Mon Sep 17 00:00:00 2001
From: David Zhang <david.zh...@highgo.ca>
Date: Fri, 19 Aug 2022 14:43:01 -0700
Subject: [PATCH]     Replace many memset calls with struct initialization

    This replaces all memset() calls with struct initialization where that
    is easily and obviously possible.
---
 contrib/pageinspect/btreefuncs.c |  3 +--
 contrib/pageinspect/ginfuncs.c   | 12 +++---------
 contrib/pageinspect/gistfuncs.c  | 12 +++---------
 contrib/pageinspect/heapfuncs.c  |  4 +---
 contrib/pageinspect/rawpage.c    |  4 +---
 5 files changed, 9 insertions(+), 26 deletions(-)

diff --git a/contrib/pageinspect/btreefuncs.c b/contrib/pageinspect/btreefuncs.c
index 9375d55e14..a14d83f8cb 100644
--- a/contrib/pageinspect/btreefuncs.c
+++ b/contrib/pageinspect/btreefuncs.c
@@ -311,7 +311,7 @@ bt_page_print_tuples(struct user_args *uargs)
        bool            rightmost = uargs->rightmost;
        bool            ispivottuple;
        Datum           values[9];
-       bool            nulls[9];
+       bool            nulls[9] = {0};
        HeapTuple       tuple;
        ItemId          id;
        IndexTuple      itup;
@@ -331,7 +331,6 @@ bt_page_print_tuples(struct user_args *uargs)
        itup = (IndexTuple) PageGetItem(page, id);
 
        j = 0;
-       memset(nulls, 0, sizeof(nulls));
        values[j++] = DatumGetInt16(offset);
        values[j++] = ItemPointerGetDatum(&itup->t_tid);
        values[j++] = Int32GetDatum((int) IndexTupleSize(itup));
diff --git a/contrib/pageinspect/ginfuncs.c b/contrib/pageinspect/ginfuncs.c
index 952e9d51a8..6c0183cc63 100644
--- a/contrib/pageinspect/ginfuncs.c
+++ b/contrib/pageinspect/ginfuncs.c
@@ -40,7 +40,7 @@ gin_metapage_info(PG_FUNCTION_ARGS)
        GinMetaPageData *metadata;
        HeapTuple       resultTuple;
        Datum           values[10];
-       bool            nulls[10];
+       bool            nulls[10] = {0};
 
        if (!superuser())
                ereport(ERROR,
@@ -75,8 +75,6 @@ gin_metapage_info(PG_FUNCTION_ARGS)
 
        metadata = GinPageGetMeta(page);
 
-       memset(nulls, 0, sizeof(nulls));
-
        values[0] = Int64GetDatum(metadata->head);
        values[1] = Int64GetDatum(metadata->tail);
        values[2] = Int32GetDatum(metadata->tailFreeSize);
@@ -107,7 +105,7 @@ gin_page_opaque_info(PG_FUNCTION_ARGS)
        GinPageOpaque opaq;
        HeapTuple       resultTuple;
        Datum           values[3];
-       bool            nulls[3];
+       bool            nulls[3] = {0};
        Datum           flags[16];
        int                     nflags = 0;
        uint16          flagbits;
@@ -162,8 +160,6 @@ gin_page_opaque_info(PG_FUNCTION_ARGS)
                flags[nflags++] = DirectFunctionCall1(to_hex32, 
Int32GetDatum(flagbits));
        }
 
-       memset(nulls, 0, sizeof(nulls));
-
        values[0] = Int64GetDatum(opaq->rightlink);
        values[1] = Int32GetDatum(opaq->maxoff);
        values[2] = PointerGetDatum(construct_array_builtin(flags, nflags, 
TEXTOID));
@@ -255,14 +251,12 @@ gin_leafpage_items(PG_FUNCTION_ARGS)
                HeapTuple       resultTuple;
                Datum           result;
                Datum           values[3];
-               bool            nulls[3];
+               bool            nulls[3] = {0};
                int                     ndecoded,
                                        i;
                ItemPointer tids;
                Datum      *tids_datum;
 
-               memset(nulls, 0, sizeof(nulls));
-
                values[0] = ItemPointerGetDatum(&cur->first);
                values[1] = UInt16GetDatum(cur->nbytes);
 
diff --git a/contrib/pageinspect/gistfuncs.c b/contrib/pageinspect/gistfuncs.c
index d0a34a3375..1e8cc88b65 100644
--- a/contrib/pageinspect/gistfuncs.c
+++ b/contrib/pageinspect/gistfuncs.c
@@ -43,7 +43,7 @@ gist_page_opaque_info(PG_FUNCTION_ARGS)
        GISTPageOpaque opaq;
        HeapTuple       resultTuple;
        Datum           values[4];
-       bool            nulls[4];
+       bool            nulls[4] = {0};
        Datum           flags[16];
        int                     nflags = 0;
        uint16          flagbits;
@@ -99,8 +99,6 @@ gist_page_opaque_info(PG_FUNCTION_ARGS)
                flags[nflags++] = DirectFunctionCall1(to_hex32, 
Int32GetDatum(flagbits));
        }
 
-       memset(nulls, 0, sizeof(nulls));
-
        values[0] = LSNGetDatum(PageGetLSN(page));
        values[1] = LSNGetDatum(GistPageGetNSN(page));
        values[2] = Int64GetDatum(opaq->rightlink);
@@ -163,7 +161,7 @@ gist_page_items_bytea(PG_FUNCTION_ARGS)
                 offset++)
        {
                Datum           values[5];
-               bool            nulls[5];
+               bool            nulls[5] = {0};
                ItemId          id;
                IndexTuple      itup;
                bytea      *tuple_bytea;
@@ -177,8 +175,6 @@ gist_page_items_bytea(PG_FUNCTION_ARGS)
                itup = (IndexTuple) PageGetItem(page, id);
                tuple_len = IndexTupleSize(itup);
 
-               memset(nulls, 0, sizeof(nulls));
-
                values[0] = DatumGetInt16(offset);
                values[1] = ItemPointerGetDatum(&itup->t_tid);
                values[2] = Int32GetDatum((int) IndexTupleSize(itup));
@@ -241,7 +237,7 @@ gist_page_items(PG_FUNCTION_ARGS)
                 offset++)
        {
                Datum           values[5];
-               bool            nulls[5];
+               bool            nulls[5] = {0};
                ItemId          id;
                IndexTuple      itup;
                Datum           itup_values[INDEX_MAX_KEYS];
@@ -258,8 +254,6 @@ gist_page_items(PG_FUNCTION_ARGS)
                index_deform_tuple(itup, RelationGetDescr(indexRel),
                                                   itup_values, itup_isnull);
 
-               memset(nulls, 0, sizeof(nulls));
-
                values[0] = DatumGetInt16(offset);
                values[1] = ItemPointerGetDatum(&itup->t_tid);
                values[2] = Int32GetDatum((int) IndexTupleSize(itup));
diff --git a/contrib/pageinspect/heapfuncs.c b/contrib/pageinspect/heapfuncs.c
index 2ff70405cf..952f5ed57e 100644
--- a/contrib/pageinspect/heapfuncs.c
+++ b/contrib/pageinspect/heapfuncs.c
@@ -181,13 +181,11 @@ heap_page_items(PG_FUNCTION_ARGS)
                Datum           result;
                ItemId          id;
                Datum           values[14];
-               bool            nulls[14];
+               bool            nulls[14] = {0};
                uint16          lp_offset;
                uint16          lp_flags;
                uint16          lp_len;
 
-               memset(nulls, 0, sizeof(nulls));
-
                /* Extract information from the line pointer */
 
                id = PageGetItemId(page, inter_call_data->offset);
diff --git a/contrib/pageinspect/rawpage.c b/contrib/pageinspect/rawpage.c
index 90942be71e..0d1292d8f1 100644
--- a/contrib/pageinspect/rawpage.c
+++ b/contrib/pageinspect/rawpage.c
@@ -252,7 +252,7 @@ page_header(PG_FUNCTION_ARGS)
        Datum           result;
        HeapTuple       tuple;
        Datum           values[9];
-       bool            nulls[9];
+       bool            nulls[9] = {0};
 
        Page            page;
        PageHeader      pageheader;
@@ -318,8 +318,6 @@ page_header(PG_FUNCTION_ARGS)
 
        /* Build and return the tuple. */
 
-       memset(nulls, 0, sizeof(nulls));
-
        tuple = heap_form_tuple(tupdesc, values, nulls);
        result = HeapTupleGetDatum(tuple);
 
-- 
2.17.1

Reply via email to