LLNL / Silo

Mesh and Field I/O Library and Scientific Database
https://silo.llnl.gov
Other
28 stars 23 forks source link

incompatible integer to pointer conversion error/warning in silo_hdf5.c #296

Closed DimitryAndric closed 1 year ago

DimitryAndric commented 1 year ago

During a recent run of the FreeBSD ports tree with clang 15.0, I was notified that Silo failed to build due to a new warning emitted by this recent version of clang:

/wrkdirs/usr/ports/science/silo/work/Silo-4.11-68-g819658e/src/hdf5_drv/silo_hdf5.c:1869:13: error: incompatible integer to pointer conversion passing 'char' to parameter of type 'const char *'; take the address with & [-Wint-conversion]
            DB_OBJ_CASE(DB_CURVE, DBcurve_mt, npts?1:1, yvarname)
            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/wrkdirs/usr/ports/science/silo/work/Silo-4.11-68-g819658e/src/hdf5_drv/silo_hdf5.c:1812:36: note: expanded from macro 'DB_OBJ_CASE'
            (*dsnames)[i] = strdup(m.MEMNAME[i]);                \
                            ~~~~~~~^~~~~~~~~~~~~
/wrkdirs/usr/ports/science/silo/work/Silo-4.11-68-g819658e/src/silo/silo_private.h:939:35: note: expanded from macro 'strdup'
#define strdup(s) _db_safe_strdup(s)
                                  ^
/wrkdirs/usr/ports/science/silo/work/.build/include/silo.h:2214:68: note: passing argument to parameter here
SILO_API extern char *                 _db_safe_strdup(const char *);
                                                                   ^

I have kept staring at that piece of code, but I am unsure what is supposed to do:

        switch(_objtype)
        {
            DB_OBJ_CASE(DB_QUADVAR, DBquadvar_mt, nvals, value)
            /*DB_OBJ_CASE(DB_QUAD_RECT, DBquadmesh_mt, nspace, coord) wont work for rect case */
            DB_OBJ_CASE(DB_QUAD_CURV, DBquadmesh_mt, nspace, coord)
            DB_OBJ_CASE(DB_QUADMESH, DBquadmesh_mt, nspace, coord)
            DB_OBJ_CASE(DB_UCDVAR, DBucdvar_mt, nvals, value)
            DB_OBJ_CASE(DB_UCDMESH, DBucdmesh_mt, ndims, coord)
            DB_OBJ_CASE(DB_POINTVAR, DBpointvar_mt, nvals, data)
            DB_OBJ_CASE(DB_POINTMESH, DBpointmesh_mt, ndims, coord)
            DB_OBJ_CASE(DB_CSGVAR, DBcsgvar_mt, nvals, vals)
            DB_OBJ_CASE(DB_CURVE, DBcurve_mt, npts?1:1, yvarname)
        }

It is complaining about the last DB_OBJ_CASE() line, which effectively expands to:

    case DB_CURVE:
    {   DBcurve_mt m; int i;
        memset(&m, 0, sizeof m);
        if ((attr=H5Aopen_name(o, "silo"))<0 ||
             H5Aread(attr, DBcurve_mt5, &m)<0 ||
             H5Aclose(attr)<0)
             *dscount = 0;
        else
             *dscount = m.npts?1:1;
        *dsnames = (char **) calloc(*dscount, sizeof(char**));
        for (i = 0; i < *dscount; i++)
            (*dsnames)[i] = strdup(m.yvarname[i]);
        break;
    }

Most of the other DB_OBJ_CASE() macros have a multidimensional last argument, but the problem is that DBcurve_mt::yvarname is declared as char yvarname[256]; so indeed, attempting to strdup(m.yvarname[i]) does not really make sense: you should pass a char pointer to strdup(), not a plain char.

Of course I could pass some compiler option to simply silence the warning, but it would seem better to fix the code to do what is right. However, I do not have a clue as to what is correct, in this case?

markcmiller86 commented 1 year ago

Thanks for reporting. It is fixed on 4.11RC in 87d47fc3812743f49438b7771d02c2a5589bdf3f.

You can see a diff here...

https://github.com/LLNL/Silo/commit/be29ddf0352bc8e5a7eecc8772a3acb64dfde18c

You can download a patch by adding .patch to the above url...

https://github.com/LLNL/Silo/commit/be29ddf0352bc8e5a7eecc8772a3acb64dfde18c.patch

So, do this...

  1. cd to wherever you have silo-4.11 source tree top dir
  2. wget https://github.com/LLNL/Silo/commit/be29ddf0352bc8e5a7eecc8772a3acb64dfde18c.patch
  3. patch src/hdf5_drv/silo_hdf5.c be29ddf0352bc8e5a7eecc8772a3acb64dfde18c.patch

Fix will be in upcoming 4.11.1 release aiming for end of Jan, 2023.

DimitryAndric commented 1 year ago

Thank you, I will submit the fix for inclusion in the FreeBSD port, until it gets updated to 4.11.1.