3

I am implementing an implementation of queues in C. My interface consists of five simple function to access the queue:

#ifndef QUEUE_H
#define QUEUE_H

#include <stdbool.h>
#include <stddef.h>

struct queue {
  struct cell* first;
  struct cell* last;
};

typedef struct queue queue;

extern queue newQueue(void);
extern bool  isEmpty(queue);
extern queue enqueue(queue,void*);
extern queue dequeue(queue);
extern void* front(queue);
extern void  freeQueue(queue);

Since two of them (newQueue and isEmpty) are so trivial that I believe that a compiler can do many good optimizations with them, I decided to write inline declarations for them:

/* replacing the two lines

extern queue newQueue(void);
extern bool  isEmpty(queue);

in the original header */

extern inline queue newQueue(void) {
  queue q = { NULL, NULL };
  return q;
}

extern inline bool isEmpty(queue q) {
  return q.first == NULL;
}

This compile fine with gcc. But when I compile it with clang, it gives me an error. A quick research shows, that the official way of doing these inline declarations is different from the GNU style. I could either pass -std=gnu89 or change the function signatures according to the link above. I chosed the second option:

inline queue newQueue(void) {
  queue q = { NULL, NULL };
  return q;
}

inline bool isEmpty(queue q) {
  return q.first == NULL;
}

But now, both clang and gcc say something about duplicate function declarations, when compiled in c99 mode. This is the accompanying definition in queue.c:

#include "queue.h"

/* ... */

queue newQueue() {
  queue q = { NULL, NULL };
  return q;
}

bool isEmpty(queue q) {
  return q.first == NULL;
}

What am I doing wrong? How can I get what I want without needing to switch into gnu89 mode?

These are the error messages I get with the second style:

$ gcc -std=c99 queue.c 
queue.c:12:7: error: redefinition of ‘newQueue’
queue.h:14:21: note: previous definition of ‘newQueue’ was here
queue.c:17:6: error: redefinition of ‘isEmpty’
queue.h:19:20: note: previous definition of ‘isEmpty’ was here
$ clang -std=c99 queue.c
queue.c:12:7: error: redefinition of 'newQueue'
queue newQueue() {
      ^
In file included from queue.c:5:
./queue.h:14:21: note: previous definition is here
extern inline queue newQueue(void) {
                    ^
queue.c:17:6: error: redefinition of 'isEmpty'
bool isEmpty(queue q) {
     ^
In file included from queue.c:5:
./queue.h:19:20: note: previous definition is here
extern inline bool isEmpty(queue q) {
                   ^
2 errors generated.
Community
  • 1
  • 1
fuz
  • 88,405
  • 25
  • 200
  • 352

3 Answers3

4

If you are defining functions in headers make them static. This should be enough for compiler to inline them (inline is just an additional hint).

Non static functions in headers will result in multiple definitions, if you include that header more than one time in your whole program.

I have done some research and have more info:

You can use inline that way. At least in C99. You just cannot have both inline and non-inline definitions in queue.c. You need wrap inline definitions in #ifdef or move them to header not included in queue.c.

You need to write the functions twice and play with the preprocessor, but it should work exactly as you want. When function is not inlined, it will be emitted only once.

Piotr Praszmo
  • 17,928
  • 1
  • 57
  • 65
  • In case of defining the function as `static`, isn't it possible that the function will end up being compiled multiple times in my executable? Is there a way to ensure that if a function is really generated, then only once? – fuz Oct 18 '11 at 20:23
  • @FUZxxl Why do you care? It's the toolset's job to generate decent enough executable. The compiler and linker can cooperate in that task. With clang/llvm, you can also do link-time optimization that will optimize the entire program, not just individual modules. Whatever gains come from this will make any "losses" due to repeated functions quite irrelevant. Or, in other words, unless you can measure that it has impact, don't sweat such things. It's not a premature pessimization. – Kuba hasn't forgotten Monica Mar 29 '14 at 02:31
  • @KubaOber Not all toolsets are smart enough to figure out that two functions in distinct translation units are equal. In most toolchains (including the GNU toolchain) such a thing is probably impossible as important information is not available (e.g. where a function ends). I don't want to code for one toolchain, I want to write portable code that works everywhere and is as efficient as possible everywhere. This excludes any assumptions about the capabilites of one specific toolchain. – fuz Mar 29 '14 at 02:46
  • If the function is `inline` then it may be copied into your executable every time it is called - that's what `inline` means. So there's no "additional harm" in making it `static inline`. – M.M Mar 29 '14 at 03:10
  • @FUZxxl Again, why do you care that there will be some duplicate functions in the output? Run upx on the executable if you must, but really, what's the big deal? – Kuba hasn't forgotten Monica Mar 29 '14 at 03:19
  • UPX is not portable. Imagine the header with the inline function is inlined into hundreds of files. Now this can quickly turn into 100 kB of wasted memory. I don't want to waste 100 kB. – fuz Mar 29 '14 at 15:39
  • You don't need to wrap anything in an `#ifdef` or define anything twice. All `queue.c` needs is a non-inline extern declaration of the inlined functions. Since the definitions are already available from the included header the functions will be emitted (once and only once). Since declarations without inline are implicitly extern, that's as simple as: `queue newQueue(void);` and `bool isEmpty(queue q);` – Michael Morris Jan 20 '22 at 09:05
1

The proper way of doing this in c99 and onward is to merely have an external declaration of an inline function in the .c file instead of a definition of it. This will force standalone code to be created for that function so that it will link properly if inlining is not possible for some reason. See: http://www.greenend.org.uk/rjk/tech/inline.html

Since functions default to extern this is sufficient:

queue.c

#include "queue.h"

/* ... */

queue newQueue();

bool isEmpty(queue q);
Community
  • 1
  • 1
Michael Morris
  • 319
  • 3
  • 13
1

You should not be declaring them in queue.c

Jim Rhodes
  • 5,021
  • 4
  • 25
  • 38
  • That's not actually true. He should not be *defining* them in queue.c, but but under the standard c inlining rules (as opposed to GCCs pre-c99 inlining behavior) you need to either have a single `extern` declaration (most conveniently in corresponding .c file), or make the inlined function `static`. I don't believe many compilers can remove duplicated code caused by `static inline` so putting `extern` declarations in queue.c is probably the best option for what he wants. – Michael Morris Mar 29 '14 at 02:36