Compile warnings for libsrc/v1hpg.c and nc_test/tst_utf8_validate.c...

[edhartnett]

With the recent merges, we have cleared almost all compile warnings in the code for gcc -Wall.

However, here's two that have been introduced:

v1hpg.c: In function ‘v1h_get_NC_string’:
v1hpg.c:307:9: warning: variable ‘padding’ set but not used [-Wunused-but-set-variable]
  size_t padding = 0, nchars = 0;
         ^~~~~~~

v1hpg.c: In function ‘v1h_get_NC_attrV’:
v1hpg.c:695:15: warning: variable ‘padding’ set but not used [-Wunused-but-set-variable]
  size_t nget, padding;
               ^~~~~~~

Easily fixed. I will put fix in my next PR.

Another warning, which has been there for a long time, is this one:

tst_utf8_validate.c:98:44: warning: null character(s) preserved in literal
 {0,"2.1.1", "1 byte  (U-00000000)",        " "},

In this case we were using '^@' for a null char in a string literal, instead of the equivalent (and gcc-preferred) '/000'. I made the change to eliminate the warning.

[gsjaardema]

Looking very nice now. I am also seeing a couple others:

In file included from ncgen.y:908:0:
ncgen.l: In function 'ncglex':
ncgen.l:388:11: warning: unused variable 'signchar' [-Wunused-variable]
       int signchar = 0;
           ^
In file included from ncgen.y:908:0:
ncgen.l: In function 'parseULL':
ncgen.l:663:9: warning: unused variable 'result' [-Wunused-variable]
     extern int errno;
         ^
ncgen.y: In function 'makespecial':
ncgen.y:1163:18: warning: unused variable 'udata' [-Wunused-variable]
     unsigned int udata =  0;
                  ^

And

[ 10%] Building C object libsrc/CMakeFiles/netcdf3.dir/putget.c.o
/Users/gdsjaar/src/seacas-sems/TPL/netcdf/netcdf-c/libsrc/putget.c: In function 'putNCvx_char_char':
/Users/gdsjaar/src/seacas-sems/TPL/netcdf/netcdf-c/libsrc/putget.c:892:15: warning: unused variable 'fillp' [-Wunused-variable]
         void *fillp=NULL;
               ^

[gsjaardema]

Clang finds a few others including:

/Users/gdsjaar/src/seacas-sems/TPL/netcdf/netcdf-c/ncdap_test/t_dap3a.c:116:12: warning: unused variable 'len' [-Wunused-variable]
    size_t len;
           ^
1 warning generated.

/Users/gdsjaar/src/seacas-sems/TPL/netcdf/netcdf-c/ncdap_test/test_cvt.c:115:12: warning: unused variable 'len' [-Wunused-variable]
    size_t len;
           ^
1 warning generated.

/Users/gdsjaar/src/seacas-sems/TPL/netcdf/netcdf-c/libdap4/d4file.c:372:13: warning: unused variable 'ok' [-Wunused-variable]
        int ok;
            ^

/Users/gdsjaar/src/seacas-sems/TPL/netcdf/netcdf-c/libdap4/d4read.c:153:10: warning: unused variable 'buf' [-Wunused-variable]
    char buf[1024];
         ^
1 warning generated.

/Users/gdsjaar/src/seacas-sems/TPL/netcdf/netcdf-c/ncgen/genc.c:520:15: warning: comparison of unsigned expression < 0 is always false
      [-Wtautological-compare]
            if(level < 0 || level > 9)
               ~~~~~ ^ ~
/Users/gdsjaar/src/seacas-sems/TPL/netcdf/netcdf-c/ncgen/genc.c:528:25: warning: comparison of unsigned expression >= 0 is always true
      [-Wtautological-compare]
                        (level >= 0?1:0),
                         ~~~~~ ^  ~
2 warnings generated.
[ 27%] Building C object ncgen/CMakeFiles/ncgen.dir/genbin.c.o
/Users/gdsjaar/src/seacas-sems/TPL/netcdf/netcdf-c/ncgen/genbin.c:252:15: warning: comparison of unsigned expression < 0 is always false
      [-Wtautological-compare]
            if(level < 0 || level > 9)
               ~~~~~ ^ ~
/Users/gdsjaar/src/seacas-sems/TPL/netcdf/netcdf-c/ncgen/genbin.c:258:25: warning: comparison of unsigned expression >= 0 is always true
      [-Wtautological-compare]
                        (level >= 0?1:0),
                         ~~~~~ ^  ~

In file included from ncgen.y:908:
ncgen.l:388:11: warning: unused variable 'signchar' [-Wunused-variable]
                    int signchar = 0;
                        ^
ncgen.l:663:9: warning: unused variable 'result' [-Wunused-variable]
    int result = 0;
        ^
ncgen.y:1163:18: warning: unused variable 'udata' [-Wunused-variable]
    unsigned int udata =  0;
                 ^
3 warnings generated.

In addition there are some "static analysis" type warnings about potentially uninitialized variables. I really appreciate the work on making the builds clean... very nice.

[gsjaardema]

I think this warning points to an incorrect test:

/Users/gdsjaar/src/seacas-sems/TPL/netcdf/netcdf-c/libdap4/d4file.c:125:9: warning: comparison of array 'tmpname' equal to a null pointer is
      always false [-Wtautological-pointer-compare]
            if(tmpname == NULL) ret = NC_ENOMEM;
               ^~~~~~~    ~~~~

The code above this is:

	    d4info->substrate.filename = strdup(tmpname);
	    if(tmpname == NULL) ret = NC_ENOMEM;

So I think is should be testing d4info->substrate.filename

[edhartnett]

Howdy @gdsjaar! Allow me to address your points...

For the ncgen.y warnings, I'm not sure what to do if anything. The .y file is part of ncgen which generates some C code. I'm wary of messing with it, so I think I will leave it alone (at least for now). If someone can suggest how to clear these warnings, I am all ears.

For the putget warning, I actually get a slightly different warning from you:

putget.m4: In function ‘putNCvx_char_char’:
putget.m4:754:15: warning: unused variable ‘fillp’ [-Wunused-variable]
 PUTNCVX(char, char)
               ^~~~~

Note that my warning points to the m4 file, yours to the C file. (Possibly you did not start from a distclean?)

The problem is in putget.m4, here:

static int
putNCvx_$1_$2(NC3_INFO* ncp, const NC_var *varp,
		 const size_t *start, size_t nelems, const $2 *value)
{
	off_t offset = NC_varoffset(ncp, varp, start);
	size_t remaining = varp->xsz * nelems;
	int status = NC_NOERR;
	void *xp;
        void *fillp=NULL;

	if(nelems == 0)
		return NC_NOERR;

	assert(value != NULL);

#ifdef ERANGE_FILL
        fillp = malloc(varp->xsz);
        status = NC3_inq_var_fill(varp, fillp);
#endif

	for(;;)
	{
		size_t extent = MIN(remaining, ncp->chunk);
		size_t nput = ncx_howmany(varp->type, extent);

		int lstatus = ncio_get(ncp->nciop, offset, extent,
				 RGN_WRITE, &xp);
		if(lstatus != NC_NOERR)
			return lstatus;

		lstatus = ncx_putn_$1_$2(&xp, nput, value ifelse(`$1',`char',,`,fillp'));

This is m4 to generate the C code. As you can see, when $1 is 'char' the fillp is not used. (Because you can't set fill value for char? I don't remember...).

So this generates the warning. The presence of ERANGE_FILL complicates things. It is turned off by default, and I have not yet tested with it on. ;-)

I think I will leave this m4 code task to someone else until I really run out of things to do first thing in the morning to warm up my programming muscles. ;-)

[edhartnett]

When I use --enable-dap I get some more warnings, some of which are easy, and I fixed. Others I have left alone.

This includes some of the warnings you are seeing in clang.

Take a look after my next PR is merged and see what is left. If you want to help figure them out, that would be great. Whatever is left are the warnings I couldn't easily fix. ;-)

[gsjaardema]

That sounds good.

[DennisHeimbigner]

The ngenl.[ch] files are produced by the Lex lexer generator tool. Similarly,
ncgeny.[ch] are produced by the Bison parsing generator tool.
These tools generate code that known to produce large numbers of warning
when compiled. Most of these warning cannot be addressed by changes in the
ncgen.l or ncgen.y file and changing the generated code is a very bad idea
since it will get overwritten the next time bison or lex is run.

[DennisHeimbigner]

1. The tmpname code is wrong and your suggested fix is correct.
2. Unused variables, etc. that are in ncgen.y (as opposed to ncgeny.c)
   should be fixable the same as any .c file. So in this case, the unused variables
   that appear in ncgen.,y can be removed. You will just need to rebuild ncgen.[ch]
   using the makeparser entry in the Makefile.
3. For the fillp problem, I ran into this an fixed it on another branch that is not likely
   to be merged anytime soon. I think the correct solution was to use this
   #ifdef ERANGE_FILL
   void* fillp = NULL;
   #endif

[DennisHeimbigner]

I am preparing a fix pr for the following files:
libdap4/d4file.c
libdap4/d4read.c
ncgen/genc.c
ncgen/ncgen.l
ncgen/ncgen.y
to fix the warnings identified in this issue.