Introduce banned.h for banned functions

[ThomasAdam]

Fvwm has an old history, and as such the string handling has been organic, and has made use of the typical functions of the day, such as strcat and strcpy. Although their n counterparts (strncpy, strncat) are considered marginally safer, some effort should go in to converting these calls across the codebase.

However, in doing this, it's probably only fair that some level of enforcement is achieved after this point so that there's no regressions. The git project has a concept of banned.h which aborts compilation if defined functions are found. Something like this:

#ifndef BANNED_H
#define BANNED_H

/*
 * This header lists functions that have been banned from our code base,
 * because they're too easy to misuse (and even if used correctly,
 * complicate audits). Including this header turns them into compile-time
 * errors.
 */

#define BANNED(func) sorry_##func##_is_a_banned_function

#undef strcpy
#define strcpy(x,y) BANNED(strcpy)

Indeed, the following should be considered for banning:

sprintf
strcat
strcpy
strncat
strncpy

... the above can all be replaced with snprintf, or strlcpy or asprintf or any combination thereof.

To undertake this work, use of coccinelle should be used to identify and rewrite the rules required.

[pm215]

You might consider using the gcc 'poison' pragma to detect uses of unwanted functions; you get a nicer error message from the compiler, I think:

$ cat /tmp/zz9.c
#include <string.h>

#pragma GCC poison strcat strcpy strncpy

void fn_using_strcpy(char *buf)
{
    strcpy(buf, "hello world");
}
$ gcc -o /tmp/zz9.o -c /tmp/zz9.c
/tmp/zz9.c: In function ‘fn_using_strcpy’:
/tmp/zz9.c:7:5: error: attempt to use poisoned "strcpy"
     strcpy(buf, "hello world");
     ^

Works with both gcc and clang (and if you care about more compilers than that you can always guard the #pragma with ifdef GNUC which both gcc and clang will satisfy).

[ThomasAdam]

Hi @pm215

Yes indeed -- however, I do have to also consider a degree of portability. My understanding from reading about the poison pragma before is that it's C99 only. Is that true? If so, I'll probably have to bite the bullet and go for something my original proposal, albeit I agree, it wouldn't be as nice as using the poison pragma.

[pm215]

The documentation doesn't say anything about #pragma being C99-only, and at least for gcc it still works fine with "-std=c89". (The _Pragma() syntax is C99-or-later, but not #pragma.)

I'm surprised you still need to cater for pre-C99, though. I just tested with

CFLAGS='-std=c89' configure && make

and the codebase doesn't actually compile as C89/C90 at the moment, because (among other things) of files using "//" style comments, which are only valid in C99 or later. So (assuming nobody's complained that their compiler couldn't handle those) you might be reasonably able to just assume and require C99 :-)

(I found the biggest portability issue to be the dependencies -- Ubuntu Focal LTS doesn't have a new enough libbson, for instance and I had to manually install the library from a later Ubuntu. If you have a system that has the dependencies it's probably also got a non-ancient gcc or clang.)

[ThomasAdam]

Cool, then I'll use the pragma.

Indeed, we've been using things like inline for ages -- so I'm not at all convinced c89 compatibility is important. However, even compiling with -std=c99 doesn't work. It probably ought to. Do you want to send through some patches to address this?

[pm215]

Well, I don't personally care whether the code base is strict C99-only. -std=gnu99, ie "C99 + GNU extensions" works. At least some of the problems with a -std=c99 compile seem to be in the system header files.

[ThomasAdam]

Well, I don't personally care whether the code base is strict C99-only. -std=gnu99, ie "C99 + GNU extensions" works. At least some of the problems with a -std=c99 compile seem to be in the system header files.

It's the sort of thing we should address. If you don't want to do that, I understand. I was trying to drum up enough interest for someone other than myself to do this sort of work...