Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
DimitryAndric opened this issue Jan 8, 2023 · 2 comments

Comments

@DimitryAndric
Copy link

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
Copy link
Member

markcmiller86 commented Jan 9, 2023

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

You can see a diff here...

be29ddf

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
Copy link
Author

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

vishwin pushed a commit to vishwin/freebsd-ports that referenced this issue Jan 10, 2023
During an exp-run for llvm 15 (see bug 265425), it turned out that
science/silo failed to build with clang 15:

  /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 *);
                                                                     ^

This turns out to be a bug in silo, which I reported upstream in
LLNL/Silo#296, and which was fixed in
LLNL/Silo@be29ddf.

PR:		268864
Approved by:	yuri (maintainer)
MFH:		2023Q1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants