Don't test for NULL before calling free(3)

[alejandro-colomar]

free(3) accepts NULL, since the oldest ISO C.  I guess the paranoid code was taking
care of prehistoric implementations of free(3).  I've never known of an implementation
that doesn't conform to this, so let's simplify this.

Remove xfree(3), which was effectively an equivalent of free(3).

Signed-off-by: Alejandro Colomar <alx@kernel.org>

I found this while merging xmalloc()+str*cpy() into xstrdup().

[ikerexxe]

The code looks good to me but I'm a little bit preoccupied about the error found by CodeQL.

[alejandro-colomar]

The code looks good to me but I'm a little bit preoccupied about the error found by CodeQL.

Yeah, I have no idea how that's failing now, but I'll have a look in case there's a bug that has been uncovered by this trivial change.

However, there seems to be a bug in the static analyzer, since a no-op change like this shouldn't trigger any changes in the analysis.

[alejandro-colomar]

I didn't check all paths that are being reported by CodeQL, but:

One of the reports is:
This 'call to strcpy' with input from tzbuf may overflow the destination.

Let's investigate it:

$ grep -rniI -e strdup -e strcpy -e str.cpy | grep tzbuf
libmisc/tz.c:43:		strcpy (tzbuf, def_tz);

A single match; it should be easy to find out if the report is correct.
Let's print the whole function:

/*@observer@*/const char *tz (const char *fname)
{
	FILE *fp = NULL;
	static char tzbuf[BUFSIZ];
	const char *def_tz = "TZ=CST6CDT";

	fp = fopen (fname, "r");
	if (   (NULL == fp)
	    || (fgets (tzbuf, (int) sizeof (tzbuf), fp) == NULL)) {
		def_tz = getdef_str ("ENV_TZ");
		if ((NULL == def_tz) || ('/' == def_tz[0])) {
			def_tz = "TZ=CST6CDT";
		}

		strcpy (tzbuf, def_tz);
	} else {
		tzbuf[strlen (tzbuf) - 1] = '\0';
	}

	if (NULL != fp) {
		(void) fclose (fp);
	}

	return tzbuf;
}

No entry in the table of definitions (def_table) is bigger than BUFSIZ, so at least this one is not a real bug.

I wonder why this has been triggered now, if there's no call to anything free(3) in this function :/

[ikerexxe]

I wonder why this has been triggered now, if there's no call to anything free(3) in this function :/

Who knows. I'll merge the changes. Thanks for your patch.

[hallyn]

Thanks - indeed the manpage also "guarantees" null safety