xavier roche's homework

random thoughts and sometimes pieces of code

Redirecting Stdio Is Hard

The Two Nasty Bugs

The nasty bugs

You know what it is like: you never saw them, but some people did, and these people are reliable, so there must be something wrong. But nobody on the engineering team managed to reproduce the issues. And the issues are random, and only happen rarely. Very rarely, such as once in a month for a given customer, or even once in a year for another one. And you know what: these bugs are horrible, because it seems you’ll never catch them. They hide, and silently trigger when you are asleep.

First bug report: Can not run an external crash reporter on Linux

This was the nicest one, actually, and it took only few days to investigate it. Basically, in case of crash, a crash reporter was supposed to be ran, and send useful information (stack traces et al.). Unfortunately the crash reporter appeared not to work correctly. It did not start. For an unknown reason (no error messages)

Fortunately, this bug was reproducible internally.

Second bug report: I see log files in my database on Windows

Yes, log files in my database. Or more precisely log files in my database binary files. I was a bit surprised, too. It seems that, randomly, some log files that were supposed to be sent to the logs were redirected to another random file, including database ones.

Obviously, the database was not happy after that.

That was the report after months of investigation: the first ones were just the usual there was a random crash and I have no clue why – I scratched everything and now it works.

These two issues were actually related, and the root cause was the way file descriptors are handled.

Oh, what are file descriptors, but he way ?

The Three Little Descriptors

All C programmers know the three standard library input/output files: stdin, stdout, stderr. These are the default standard input, output, and error streams. The first one typically receives the console input (keyboard), the later ones allows to display output.

If you look at the stdin manual page, you will see that

A file with associated buffering is called a stream and is declared to be a pointer to a defined type FILE. (…) At program start-up, three streams shall be predefined and need not be opened explicitly: standard input (for reading conventional input), standard output (for writing conventional output), and standard error (for writing diagnostic output).

Basically, these FILE streams are encapsulating a file descriptor, that is, a lower-level input or output resource (which is generally identified by an integer, ie. an int), and provide additional features, such as buffering, and offer a set of advanced functions, such as fprintf to print formatted output.

By default, the input/output streams are connected to the console (Linux, or DOS), but you may redirect them easily – this feature is especially known in the Linux/Un*x world:

1
ls -l /tmp/foo >/tmp/list 2>/tmp/errorlog

In this very basic shell script example, the ls -l /tmp/foo command standard output is redirected to the file /tmp/list, and the standard error output is redirected to /tmp/errorlog. Note that the >/tmp/list can also be written as 1>/tmp/list: the ‘1’ and ‘2’ refer to the file descriptors 1 and 2, which are the standard output and error file descriptors. The standard input is the file descriptor 0:

1
read A 0</tmp/bar

Why are the standard input/output streams identified by 0, 1 and 2 ? Well, this is just a convention – and, by default, file descriptors are allocated increasingly, and the first three files opened (this is done by the system) are these streams.

We can redirect the standard streams easily using shell commands – but can we do that programmatically, especially for the output streams ?

Redirection to a File

We have two output file descriptors (1 and 2), and we want to capture them in a file – say, a log file. This is quite convenient, especially for server applications: log files can then be rotated, inspected…

The standard way to reassign a file descriptor is through the dup2 standard library function. This function takes an opened file descriptor, and clone it to the second file descriptor provided. You can also extract the file descriptor beneath a FILE stream through the fileno standard library function: here’s a simple code to redirect the standard output to a file:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <assert.h>

/** Redirect stdout and stderr. Returns 0 upon success. **/
static int redirect_std_out_err(const char *log_name) {
  int failure = 1;

  /* Open log FILE stream. */
  FILE *const fp = fopen(log_name, "wb");
  if (fp != NULL) {

    /* Extract log FILE fd, and standard stdout/stderr ones. */
    const int log_fd = fileno(fp);
    const int stdout_fd = fileno(stdout);
    const int stderr_fd = fileno(stderr);
    if (log_fd != -1 && stdout_fd != -1 && stderr_fd != -1) {

      /* Clone log fd to standard stdout/stderr ones. */
      if (dup2(log_fd, stdout_fd) != -1 && dup2(log_fd, stderr_fd) != -1) {
        failure = 0;
      }
    }

    /* Close initial file. We do not need it, as we cloned it. */
    fclose(fp);
  }
  return failure;
}

int main(int argc, char **argv) {
  /* Redirect stdout and stderr to a single log file */
  if (argc != 2 || redirect_std_out_err(argv[1])) {
    return EXIT_FAILURE;
  }

  /* Print output to both streams */
  printf("Hey, stdout is redirected!\n");
  fprintf(stderr, "Hey, stderr is also redirected!\n");

  return EXIT_SUCCESS;
}

And that’s it! Simple, isn’t it ?

Here Comes The Problems

Redirection is what we typically do in my company – I even modified some of the involved code. But sending output to a log file has its drawbacks. Especially regarding log file size: the file is growing, and we need to clear it time to time, or the disk will eventually fill. This is generally done through a “rotate” operation, that is:

  • closing the current log file
  • renaming it with some timestamp
  • opening a fresh one
  • cleanup old logs if necessary

File descriptors, more generally, are special resources in most operating systems, as they are inherited when spawning a process. By default, all file descriptors are transmitted to spawned processes (on POSIX systems, fork clone them, and exec functions keep them opened by default), and this is a pain: pipes and sockets are also inherited. This may lead to the following case:

  • main program opens a socket to a remote location
  • spawning a process, which inherits this socket (file descriptor) too
  • main program sens commands to remote location and closes the socket
  • spawned program still lives, and still has the socket file descriptor copy opened
  • remote location does not “sees” the client close() and is stuck

The thing is that spawned processed do not give a damn about already opened file descriptors (except for stdio). But the standard is broken for historical reasons, very unfortunately.

The solution is painful: you need to flag all possible opened file descriptors with the FD_CLOEXEC flag set. This flag explicitly tells the system that the file descriptor shall not be inherited when spawning a process:

  • open(..., ...) => open(..., ... | O_CLOEXEC)
  • fopen(..., ...) => open(..., ... | O_CLOEXEC) + fdopen(...) OR fopen(..., ... + "e")
  • dup(...) => fcntl(..., F_DUPFD_CLOEXEC, (long) 0)
  • dup2(..., ...) => dup3(..., ..., O_CLOEXEC)
  • opendir(...) => open(..., O_RDONLY | O_DIRECTORY | O_CLOEXEC) + fdopendir(...)
  • pipe(...) => pipe2(..., O_CLOEXEC)
  • acept(..., ..., ...) => accept4(..., ..., ..., O_CLOEXEC)
  • socket(..., ..., ...) => socket(..., ... | SOCK_CLOEXEC, ...)
  • and so on…

Some of these functions are non-standard (such as accept4) unfortunately. Some other will not be available (Windows). Some old kernels (Linux) will sometimes not support these features – and some old glibc will not work either (especially for the “e” flag). Yes, this is a real pain.

Oh, one last thing: do not dare to use fcntl(fd, F_SETFD, ...) to clear the FD_CLOEXEC flag: this is insanely hopeless in a multithreaded environment. Can you guess why ?

So What Was That All About ?

Okay, back yo our nasty bugs!

  1. The first one was quite easy to investigate.

Here are the investigation steps, ordered temporally:

  • “can not run an external crash reporter on Linux”; ie. can not fork()/execv() the external crash reporter in a signal handler, with a status code returned by waitpid() reporting a signal 6 caught during launch (ie. the launched program called abort())
  • we can not run the crash reporter outside the signal handler either
  • we can not run the crash reporter using system() either
  • we can not run “/bin/echo foo” through system() either; the command returns the status “return code 1”!
  • … BUT we CAN run “/bin/echo -n” through system()!

At this stage, it is clear that the issue is related to the output file descriptors.

Further investigation showed that:

  • we redirected output to logs, opening the log file with the FD_CLOEXEC flag set, AND using the Linux-specific dup3() call
  • as a result or dup3(), standard streams had their FD_CLOEXEC flag set
  • spawning an external program led to have the standard streams closed
  • attempting to write on the closed standard streams led to either a broken pipe signal, or an assertion failure depending on the program involved

The solution was simply to use the default dup2() function for standard streams. These are the only streams we want to be inherited, after all!

  1. The second bug took a bit more time

The biggest issue is that the bug could not be reproduced at first internally. Until our Q&A team managed to reproduce it (these guys are really good at finding bugs, I must admit) three days in a row.

The logs reports were strange, and totally unrelated. But one log was kind of interesting:

1
2
3
[2014/11/28-10:59:08.514] [info] ...
[2014/11/28-10:59:08.659] [info] ...
[Fatal Error] :2:999961: XML document structures must start and end within the same entity.

The error was a corrupted produced XML document, with another unrelated log inside it, in the middle of the structure. The final error did not have any timestamp, but the previous one was just before midnight (GMT). Humm, a midnight bug ? What are the chances for this being a coincidence ?

I then realized that midnight was the trigger time for a number of maintenance operations, including… log rotation.

But after thoughtfully inspecting the code, everything looked fine. And the bug never popped up on Linux, it was Windows-specific (or at least, much frequent on this operating system)

But there was another clue: all weird logs were actually logs coming from the Java virtual machine (either through Log4j, or through System.out.println() calls)

When using System.out (or System.err) in Java, we are actually using a PrintStream, which is an object wrapped in various FilterOutputStream objects for buffering, the final instance being a FileOutputStream, which is, according to the documentation an:

output stream for writing data to a File or to a FileDescriptor

And, in our case, the FileDescriptor involved was FileDescriptor.out and FileDescriptor.err, that is, Java handles to the standard output and error streams.

Fortunately, the sources of the Open JDK are public, and it is possible to dig into the real code beneath, that is, the native code:

  • ./jdk/src/share/classes/java/io/FileOutputStream.java
  • ./jdk/src/windows/native/java/io/FileOutputStream_md.c
  • ./jdk/src/solaris/native/java/io/FileOutputStream_md.c

Hummm… there is a Windows-specific implementation for FileOutputStream native calls. strange, isn’t it ?

1
2
3
4
JNIEXPORT void JNICALL
Java_java_io_FileOutputStream_write(JNIEnv *env, jobject this, jint byte, jboolean append) {
    writeSingle(env, this, byte, append, fos_fd);
}

And the writeSingle call is using a file descriptor returned by GET_FD():

1
2
3
4
5
6
void
writeSingle(JNIEnv *env, jobject this, jint byte, jboolean append, jfieldID fid) {
    // Discard the 24 high-order bits of byte. See OutputStream#write(int)
    char c = (char) byte;
    jint n;
    FD fd = GET_FD(this, fid);

And this macro just calls GetObjectField(..., IO_handle_fdID) to retrieve … the “handle” member of the corresponding FileDescriptor class.

What ? A handle ? Not a file descriptor ?

Yep: a HANDLE – and this was the root cause of all out problems.

Let me give you the reason: on Windows, a standard library handle is actually a kernel HANDLE wrapped by the C library, nothing more.

It means that if you replace the standard library input/output streams, you’ll just replace the C library streams.

Here is the complete bug scenario on Windows after collecting all the clues:

  • log rotation function is called at low level, and replaces the standard output and error streams 1 and 2 using dup2()
  • the Visual C++ C library replaces the streams, by closing the underlying existing HANDLE, and replacing by new ones
  • the old HANDLE are now totally meaningless (“dangling handles”)
  • Java has no knowledge of the replacement, and still has the old HANDLE stored in its FileDescriptor internals
  • Java System.out calls are now sending data to nowhere (this is bad)
  • someone opens a new file, and Windows recycle the old HANDLE
  • Java System.out calls are now sending data to a valid — yet unrelated — file (this is very very bad)
  • data corruption ensues (and not profit)

The root cause is that Java is unaware that the low-level HANDLE has changed.

The solution is simple: let’s tell him!:

For each System.out and System.err OutputStream instances, we need to:

  • get the “out” member field of the FilterOutputStream class and mark it public (see Field.setAccessible())
  • unwrap all FilterOutputStream wrapping by calling fetching the “out” member field recursively
  • cast the resulting instance to a FileOutputStream
  • fetch the FileDescriptor through FileOutputStream.getFD()
  • use SharedSecrets.getJavaIOFileDescriptorAccess() to set the HANDLE (a long) value to -1 temporarily
  • rotate the logs
  • fetch the new HANDLE (a long) through a native call to _get_osfhandle()
  • set the new HANDLE using the previous steps

This is a bit cumbersome – but mixing native code and Java is always cumbersome, so you have to be used to it :)

Epilogue

The two issues were rather different:

  • one was a mistake created when attempting to fix the insane default handle inheritance on both POSIX and Windows API, causing external programs not to run anymore
  • one was a design bug between the Visual C++ library and the JVM (ie. there is no dup2() for HANDLEs), causing random corruptions on files or sockets

But in both cases, a simple standard stream mishandling was the root cause. But the consequences were nasty, the resolution painful, and the time spent unreasonable (the second bug took months to be investigated)

TL;DR: redirecting Stdio is Hard, and prone to nasty bugs if you are not careful enough!

I Found a Bug in Strncat()

Yes, A Bug

This code won’t behave correctly (ie. partial copy) on many glibc versions on Linux:

1
strncat(dest, source, SIZE_MAX);

This is a corner case, but this is a bug.

(hint: the correct behavior is to do strcat(dest, source) equivalent)

A Secure Version Of Strcat

It began with code cleanup.

In httrack, there is a lot of legacy (old, dirty) code laying around. Rewriting everything from scratch would probably be a sane decision, but I just don’t have the time (and the courage) to do that, so I’m cleaning up bits and bits when possible, attempting to improve the code quality when possible.

One of the terrible design decision was to use C strings everywhere, and associated strcpy, strcat etc. primitives. It was an unfortunate decision, because of the lack of flexibility (immutable capacity), and security issues (buffer overflows that might be introduced).

In this hypothetical case, we’re adding a string to a structure foo:

1
2
3
4
5
6
7
8
struct foo {
  char filename[1024];
...
};

void append_filename(struct foo *foo, const char *filename) {
  strcat(foo->filename, filename);
}

This code will overflow if the passed string added to the existing string overflows the filename buffer.

The strlcat version exists on BSD, and is a reasonable solution to mitigate this problem. Unfortunately, some people do not like BSD, and decided that these functions will never be part of the glibc. (insert flame here)

Besides, switching to strlcat and its friends would involve checking every single occurrence of strcat, strcpy. etc., probably introducing new bugs!

I then decided to use a modified version of the dangerous libc primitives, using compile-time type checking:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
/**
 * Check whether 'VAR' is of type char[].
 */
#if (defined(__GNUC__) && !defined(__cplusplus))
/* Note: char[] and const char[] are compatible */
#define HTS_IS_CHAR_BUFFER(VAR) ( __builtin_types_compatible_p ( typeof (VAR), char[] ) )
#else
/* Note: a bit lame as char[8] won't be seen. */
#define HTS_IS_CHAR_BUFFER(VAR) ( sizeof(VAR) != sizeof(char*) )
#endif
#define HTS_IS_NOT_CHAR_BUFFER(VAR) ( ! HTS_IS_CHAR_BUFFER(VAR) )

/**
 * Append at most N characters from "B" to "A".
 * If "A" is a char[] variable then the size is assumed to be the capacity of this array.
 */
#define strncatbuff(A, B, N) \
  ( HTS_IS_NOT_CHAR_BUFFER(A) \
  ? strncat(A, B, N) \
  : strncat_safe_(A, sizeof(A), B, \
  HTS_IS_NOT_CHAR_BUFFER(B) ? (size_t) -1 : sizeof(B), N, \
  "overflow while appending '" #B "' to '"#A"'", __FILE__, __LINE__) )

The idea was to sniff at compile-time the arguments passed to the modified strncat version: when an opaque char* is used, there’s nothing you can do. But when passing a regular char[...] array, a secure version can be used. This is not perfect (especially because with non-GCC compilers, char[8] are seen as char*), but at least it allows you to quickly mitigate many potential buffer overflows without touching existing code.

But at one point I did this quite daring thing to create the secure strcat version:

1
#define strcatbuff(A, B) strncatbuff(A, B, (size_t) -1)

The (size_t) -1 value was simply passed as the boundary of strncatbuff – at some point, we would be doing strncat(dest, source, (size_t) -1), and this seemed fine to me.

Soon after, bugs started to appear (strings were copied partially). It took me a bit of time (with the kind help of the bug reporter) to figure out that my daring macro was involved in this bug. This took me a while because the bug only appeared on Linux, on some glibc releases, and quite randomly.

I managed to have a reproducible minimalistic case:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
/* strncat_size_t-1_testcase. */

/* gcc -Wall strncat_size_t-1_testcase.c -o strncat_size_t-1_testcase */
#include <stdio.h>
#include <stdlib.h>
#include <stdint.h>
#include <string.h>
#include <assert.h>

/* strncat_size_t-1_testcase "Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod tempor" */
int main(int argc, char **argv) {
  const size_t size = strlen(argv[1]);
  char *const buffer = malloc(size + 1);
  int success;
  buffer[0] = '\0';
  strncat(buffer, argv[1], SIZE_MAX);
  success = strlen(buffer) == size;
  if (!success) {
    fprintf(stderr, "** result: '%s'\n", buffer);
  }
  assert(success);
  return success ? EXIT_SUCCESS : EXIT_FAILURE;
}

If you build this program, and use it:

1
2
gcc -Wall strncat_size_t-1_testcase.c -o strncat_size_t-1_testcase
./strncat_size_t-1_testcase "Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod tempor"

… then you’ll get an error. Or maybe you’ll get one. Randomly.

This Is Not A glibc Bug, But A Misuse Of Strncat

The only way to be sure was to carefully read the strncat standard.

The strncat() function shall append not more than n bytes (a null byte and bytes that follow it are not appended) from the array pointed to by s2 to the end of the string pointed to by s1. The initial byte of s2 overwrites the null byte at the end of s1. A terminating null byte is always appended to the result. If copying takes place between objects that overlap, the behavior is undefined.

With the help of specialists in comp.unix.programmer, several points must be noticed:

  • strncat is not a secure version of strcat (strlcat is), and the third argument is not the destination capacity
  • using (size_t) -1 as third argument is actually using SIZE_MAX
  • the third argument is an additional limit and can have any value

Therefore, strncat(..., ..., SIZE_MAX) should behave as strcat(..., ...).

Yes, yes, this is a corner case. But this is a legit one.

Where Is The Bug ?

The reason why not all glibc releases were impacted is simple: this is an optimization bug. (insert laughs from experienced developers)

The original strncat source is not used on x86-64 architectures: the optimized SSE2 version is used in place.

The code might have been introduced in 2013 in this commit which was supposed to introduce a “Faster strlen on x64” – but some existing files seems to have been merged at that time.

Unfortunately, the assembly source is uncommented, and I did not have the courage to dig deeper. But this source is buggy.

Funny Remark: By the way, I’m not even convinced that the assembly version is faster at all: code executing speed is probably irrelevant compared to L1/L2/L3/memory bandwidth issues.

What’S Next ?

First I obviously stopped using SIZE_MAX with strncat, and dispatch to strcat accordingly. This was a daring thing to use this corner case value anyway, prone to … corner-case bugs.

The second step was to fill a bug report in the glibc.

TL;DR: Beware when using corner cases, even when perfectly conforming to the standard. And never “optimize” code by rewriting obvious functions into a 300-lines of undocumented assembly!

Coucal, Cuckoo-hashing-based Hashtable With Stash Area C Library

Greater Coucal Centropus sinensis

Coucal ?

According to Wikipedia,

A coucal is one of about 30 species of birds in the cuckoo family. All of them belong in the subfamily Centropodinae and the genus Centropus. Unlike many Old World cuckoos, coucals are not brood parasites.

If you read my previous blog entry on HTTrack’s new hashtables, you probably already know that cuckoo hashing is a rather new (2001) hashing technique to be used for efficient hashtables.

The idea is to have a smarter open-addressing strategy based on kicking an existing entry if a collision occurs. This is why the name Cuckoo hashtable was chosen.

Note that the name is a bit misleading: a typical cuckoo will not only kick an existing foe, but will consequently kill it (nature, you scary!). Here, we only relocate it, reason why I choose the Coucal, a nicer cuckoo member who does not destroy existing entries – I mean, existing eggs!

This implementation has the following features:

  • guaranteed constant time (Θ(1)) lookup
  • guaranteed constant time (Θ(1)) delete or replace
  • average constant time (O(1)) insert
  • one large memory chunk for table (and one for the key pool)
  • simple enumeration

This implementation has been tested since July 2013 in HTTrack, and so far, no bugs were reported (the only issues spotted were bugs in upstream code that was corrupting existing keys)

Please have a look at this library implementation: https://github.com/xroche/coucal

TL;DR: cuckoo hashtables are both efficient and smart!

C Corner Cases (and Funny Things)

Enter The void

void is a very strange animal. Basically void means nothing, emptiness. A function “returning” void actually does not return anything. On the machine code side, register(s) aimed to hold return values are simply unused in such case.

You won’t be surprised to hear that the size of void is undefined for this reason ; ie. sizeof(void) is not supposed to exist, even if some compilers have rather dirty extensions that allow to set this size to 1 to do pointer arithmetics.

But what about void* ? A pointer to something that does not exist, what’s that ?

Well, this is the first trap: void* is not really related to void. The void* type is used to store addresses of unspecified types ; ie. you can cast from/to the void* type, for example when passing generic functions (such as read(), write(), memset()..)

When I say cast, I don’t really mean cast, actually. You can view the pointer to void as a kind of a supertype, and aliasing rules do not apply per se: casting int* to void* is legit and vice-versa. But casting the resulting void* pointer to long*, for example, is not, because aliasing rules would be violated by indirectly casting from int* to long*. Yes, aliasing rules are a whole lot of fun. We’ll see that later.

Functions Are Not Data

Here comes another trap: you can cast anything from/to void*, except pointer to functions. Yep, in C, pointers to data and functions are not necessarily if the same size. Basically this means that the address space dedicated to data (data/bss segments) and the one dedicated to code (text segment) are not necessarily of the same size (you could have a 32-bit code pointers and 64-bit data pointers for example). Of course this is almost always the case, because the universe would otherwise collapse, and standard functions such as dlsym would not work anymore.

To quote the holy standard:

The ISO C standard does not require that pointers to functions can be cast back and forth to pointers to data. Indeed, the ISO C standard does not require that an object of type void * can hold a pointer to a function. Implementations supporting the XSI extension, however, do require that an object of type void * can hold a pointer to a function.

Function Pointer Definition

Oh, yes.

1
  int (*function)(char* s, int c) = my_function;

Function definitions are sometimes see as tricky by newcomers. They are NOT. Simply replace (*function) on the left member (the parenthesis are there for operator priority reasons) by a function name, and you just have the classic function declaration.

Is char Nice ?

In comparison to void* and function pointers, char is the nice guy:

  • its size is 1 by definition, which makes the pointer variant suitable for pointer arithmetics
  • it can be also cast from and to any other type without violating aliasing rules (still per se, re-casting to a third incompatible type would still violate aliasing rules)
  • it can be dereferenced after a cast (ie. you can actually reads the bytes within an int* safely)

But char Is Generally Signed

Yes, generally, because this is not specified in the standard. This is actually annoying, because when handling strings, you may end up with negative characters for ASCII > 127, and negative values can be promoted to integers.

Here’s a buggy example:

1
2
3
4
5
6
7
/** Return the next character within a \0-terminated string, or EOF. **/
int my_read_char(const char *buffer, size_t *offs) {
  if (buffer[*offs] != '\0') {
    return buffer[*offs++];  /* here's the trap */
  } else {
    return EOF;
  }

This function will return negative values for ASCII > 127, and especially the value -1 for ASCII 255 (0xFF), which is also the value of EOF.

You need to explicitly cast to a unsigned char to have a working version:

1
2
3
4
5
6
7
/** Return the next character within a \0-terminated string, or EOF. **/
int my_read_char(const char *buffer, size_t *offs) {
  if (buffer[*offs] != '\0') {
    return (unsigned char) buffer[*offs++];
  } else {
    return EOF;
  }

You May Use T* for const T*, But Not T** for const T**

The typical example is that:

  • a char* string may be implicitly cast into const char*:
1
2
  char *foo = strdup("Hello, world!");
  const char *bar = foo;  /* No need for a dirty cast. */

.. but a char** array of string may NOT be implicitly cast into const char**:

1
2
3
  char *foo[2];
  foo[0] = foo[1] = strdup("Hello, world!");
  const char **bar = foo;  /* Warning. */

The compiler will complaint:

1.c:6:22: warning: initialization from incompatible pointer type [enabled by default] const char bar = foo; / Warning. /

The reason behind of the same reason why, in Java, you can not cast List<String> into List<Object>: a function taking the supertype array version may add an incompatible type without being noticed.

Here’s an example:

1
2
3
4
5
6
7
8
static void replace_the_first_element(const char **array) {
  static const char *hello = "hello, world!";
  array[0] = hello;  /* replace first element by a constant string */
}
...
  char *foo[2];
  foo[0] = foo[1] = strdup("Hello, world!");
  replace_the_first_element(foo);

In this example, without a cast warning, the function replace_the_first_element would silently replace the first element of foo by a non-constant string, and the caller would have a constant string within the array when the function returns. The code is strictly equivalent to:

1
2
3
  static const char *hello = "hello, world!";
  char *foo[2];
  foo[0] = hello;  /* now you can see the problem */

I would say that we should be able to silently cast T** into const T*const* (an array of constant arrays of T), but anyway …

An Array Is Not Always An Array

Quizz time: what the test function below is supposed to print ? (don’t cheat!)

1
2
3
4
5
static void test(char foo[42]) {
  char bar[42];
  printf("foo: %zu bytes\n", sizeof(foo));
  printf("bar: %zu bytes\n", sizeof(bar));
}

Well, you might be surprised to see:

1
2
foo: 8 bytes
bar: 42 bytes

Yes, an array of type T in a function argument list is strictly equivalent of declaring the type as T*. This is why nobody uses this confusing syntax within function argument list.

To quote the standard:

“A declaration of a parameter as ‘array of type’ shall be adjusted to ‘qualified pointer to type’, where the type qualifiers (if any) are those specified within the [ and ] of the array type derivation.”

This also imply that the passed object is passed by reference, not value.

… and this is totally inconsistent with the handling of structures as function argument, or as return type. Yes, yes, historical mistakes.

Switch And Case Are Hacks

switch can be seen as a dispatch function, and case as label. Each switch must have its case, but you are free to interleave loops of you want, such as in:

1
2
3
4
5
6
7
switch (step) {
  while (1) {
    case step_A:
    case step_B:
      ...
   }
}

See my previous An Unusual Case of Case (/switch) entry for more fun!

Pointer To Arrays

Let’s say I have a char foo[42] and I want a pointer to this array in bar. I can write:

  • bar = foo
  • bar = &foo
  • bar = &foo[0]

What shall I use ? The first and third pointers are identical: they are char* ones, the one you generally want to use: the pointer points to a char area of undefined size. The second one is a pointer to the array of 42 chars itself. You may then use:

  • char *bar = foo;
  • char (*bar)[42] = &foo;
  • char *bar = &foo[0];

The pointer to the array of 42 chars syntax is a bit weird for newcomers, but it is logical: *bar is an array of 42 chars, so you just have to replace foo by (*bar) in the standard definition to have the correct syntax (the parenthesis are needed for operators priority reasons, otherwise you’ll declare an array of 42 pointer to char). (This trick is also helpful to understand the apparent obscure pointer to function syntax.)

The advantage of the second definition is that you still have a pointer to an array of 42 char: sizeof(*bar) is 42, *bar is of type char[] (suitable for specific compiler optimizations and safe string operations)

Empty Structure

1
struct { } foo;

this code is undefined in C: an empty structure is not officially supported. Some compilers such as GCC support them, and define the size of the structure as 0.

It is supported in C++ (because many classes do not have any member within) and in such case the sizeof of such empty structure is … 1, because C++ needs a different pointer for every instance of a given class. Oh, and because you need to do pointer arithmetics also, and having a null size would cause issues.

Strings Are char Arrays, And Are Not Const (But They Really Are)

Another corner case: string literals, such as "hello world" are actually arrays of char, with a terminating \0. The two definitions are equivalent:

1
char *foo = "hi";
1
2
static const char hi[] = { 'h', 'i', '\0' };
char *foo = (char*) hi;

Note that sizeof("hi") is equal to 3 (an array of 3 chars).

Oh, did you notice the (char*) in the second example ? Isn’t it a bit dirty ?

  • yes, it is
  • in C, string literals are unfortunately of type “array of char” and not “array of const char”
  • in reality, the type is really “array of const char”: if you dare to write inside a string literal, chances are you’ll segfault, because the string literal is inside a global read-only data segment

Parenthesis Have Sometimes Different Meanings

Let’s have a look at two code samples:

1
2
int i = 0, j = 0;
foo(i++, j+=i, j++);
1
2
int i = 0, j = 0;
bar((i++, j+=i, j++));

The first sample calls a function named foo taking three integers as arguments. Is has an undefined behavior: side-effects of i and j will be executed at an undefined time, and might even be optimized. You basically don’t have a clue of what foo will receive as arguments.

The second sample calls a function named bar taking one integer argument. It has a perfectly defined behavior. This integer is the result of the comma operator i++, j+=i, j++, which can be rewritten as the pseudo-C++-code:

1
2
3
4
5
6
7
8
9
static int my_comma(int &i, int &j) {
  i++;
  j+=i;
  return j++;
}
...

int i = 0, j = 0;
bar(my_comma(i, j));

The comma operator evaluates each expression starting from the leftmost member to the rightmost member, and yields the last member as value. All other values are discarded (each expression should yield void, actually). And between each member evaluation, a sequence point is committed: side-effects of each expression are committed for the next expression. This is why this expression has a perfectly defined behavior.

Logical And/Or Can Replace If

Two pretty nice feature of the “boolean and” (&&) and the “boolean or” (||) operators is that they:

  • commit side-effects between each operation: if (foo[i++] != '\0' && bar[i++] != '\0') is perfectly defined, as the post-increment of i will be committed before the evaluation of the right expression (ie. a sequence point exists between left and right operator evaluation)
  • these operators are short-circuit operators, which means that there is a guarantee that the right member is only evaluated if the result of the left member evaluation could not provide the result of the expression ; which allows to write something like if (foo != NULL && *foo != '\0') without fearing of a NULL pointer dereferencing

Beware of char buffer[1024] = ""

Not only is it a bit dirty to have large buffers on the stack (I must admit that some legacy code in httrack is filled with that, cough cough), but the C standard ensure that when initializing a structure or an array, missing elements shall be initialized to zero.

This basically means here that the first element of the array is explicitly initialized to zero (this is the terminating \0 of the empty string), and the 1023 other elements will be implicitly initialized to zero, too. The performance impact is the same as memset(buffer, 0, sizeof(buffer)).

Use at least an explicit buffer[0] = '\0' without any initializer when declaring the buffer.

The Bogus strchr (and Friends) Definitions

Did you notice that ?

1
2
3
char *strchr(const char *s, int c);
char *strstr(const char *haystack, const char *needle);
...

All these function takes a const char* and return a char*. How can a function transform an array of constant chars to a non-constant one ?

Well, obviously this is impossible, and the only reason of this oddity was to offer a somewhat generic function that would work for both char* and const char* strings: accepting const char* as argument will also accept char* versions by silently casting it, and returning char* will also allow to store the result in a const char* for the same reason.

Unfortunately, this may lead to:

1
2
3
4
5
char *pos = strchr(my_const_string, ' ');
if (pos != NULL) {
  *pos++ = '\0';  /* oops, I forgot that the string was constant, and the compiler did not warn me! */
  /* kaboom here */
}

A sane decision would be to split all these string handling functions into const and non-const ones. In C++, you can have specialized versions.

Never Divide By Zero .. Or Divide INT_MIN by -1 After Midnight

Dividing an integer by zero leads to a floating point exception (Yes, this is not a floating point operation, but the error triggers one.).

But dividing the smallest signed integer by -1 also leads to the same floating point exception. This is a bit of a curiosity: when dividing by -1 the smallest integer (-2147483648), the result should be 2147483648, but is too large (by one) to be represented in an integer (the largest integer is 2147483647), and the divide operation then triggers the same exception that the one triggered by the division by zero.

However, multiplying the smallest signed integer by -1 does not trigger anything because… well, because the multiply operation does not raises anything, even when overflowing.

Therefore, the first printf below will gently print a result, but the second one won’t, and the program will abort:

1
2
3
4
5
6
7
8
9
/* Note: using volatile to prevent the div to be optimized out */
volatile int int_min = INT_MIN;
volatile int minus_1 = -1;
int main(int argc, char **argv) {
  printf("%d\n", int_min * minus_1);
  printf("%d\n", int_min / minus_1);

  return EXIT_SUCCESS;
}

By the way, what’s the first result ? Well, -2147483648, because the 32-bit multiply operation of two 32-bit numbers provides a 64-bit result, which is 2147483648 (0x0000000080000000 in hexadecimal), and the lowest part is just truncated to the 32-bit 0x80000000 value, which is… -2147483648 in two’s complement arithmetics. Basically the overflow was silently ignored. Got it ?

Macros Are Not Functions

Macros are often considered evil (and will even cause C++-fans to scream and have foam on their mouth), especially when hiding multiple argument evaluation:

1
#define min(a, b) ( (a) < (b) ? (a) : (b) )

This function will not behave well when used with something like min(++i, ++j): a second increment will be committed in either i or j…

Another difference between a function version and its macro version are the side effects, which are committed before and after calling a function, but not a macro.

Preprocessor Magic

You know that __FILE__ is a magic preprocessor variable holding the current source filename, and __LINE__ the line number at the time of the macro evaluation. This is quite convenient to print debugging:

1
2
3
4
5
6
#define PRINT_CURRENT_FILE_AND_LINE printf("at %s:%d\n", __FILE__, __LINE__)
...
int main(...) {
...
  PRINT_CURRENT_FILE_AND_LINE;  /* print some debug */
...

The __FILE__ and __LINE__ macros will be expanded to the location in the main() function, at expansion time.

You can use the expansion time behavior to convert the __LINE__ numerical token to a string one, for example using a double macro expansion:

1
2
3
4
5
6
7
8
#define L_(TOKEN) #TOKEN
#define L(TOKEN) L_(TOKEN)
#define FILE_AND_LINE __FILE__ ":" L(__LINE__)

int main(int argc, char **argv) {
  printf("we are at: %s\n", FILE_AND_LINE);
  return 0;
}

A call to FILE_AND_LINE will form a string using the three substrings (in C, “foo” “bar” is equivalent to “foobar”). We need an intermediate L_() macro because otherwise __LINE__ would not be expanded, and the resulting string would simply be … "__LINE__".

Aliasing Rules

My headache is killing me, so I’m going to redirect you to the excellent article on cellperformance. But aliasing rules are very, very, very fun, trust me!

More ?

Do not hesitate to comment this article to complete it! There are other C-corner cases I probably didn’t mention…

TL;DR: corner cases are not reserved to C++!