ifdef is harmful

amitbern · 12-05-2008, 05:55 AM

Hi,
I need to write a simple function which will work only when a specific definition is defined, but i can't use ifdef in C file according to linux coding guidelines
so i wrote this example but i keep getting this error:
file.c: error: redefinition of 'function1'
file.h: error: previous definition of 'function1' was here

in this example all i want is that function1 will be called only if RUN_FUNC1 is defined, else i want the inline function1 to be called.

file.h
...
#ifdef RUN_FUNC1
void function1(void);
#else
static inline void function1(void)
{
printk("RUN_FUNC1 not defined\n");
};
#endif
...

file.c
...
void function1(void)
{
printk("RUN_FUNC1 defined\n",);
}
...

any ideas?
10x

ErV · 12-05-2008, 07:34 AM

The first thing that comes to mind is to declare RUN_FUNC1 as global variable (0 if not defined, anything else - if defined) and use plain if/else inside function. But that won't be possible in all situations.
I'd take a look at any project that is written according to linux coding standarts - to see how they dealt with similar problems.

paulsm4 · 12-05-2008, 09:46 AM

Try something like this:

Code:

file.h
...
#ifdef RUN_FUNC1_INLINE
static inline void function1(void)
{
  printk("RUN_FUNC1_INLINE defined\n");
}
#else
void function1(void);
#endif /* RUN_FUNC1_INLINE */
...

file.c

Code:

...
#ifndef RUN_FUNC1_INLINE
void function1(void)
{
  printk("RUN_FUNC1 not defined\n",);
}
#endif /* RUN_FUNC1_INLINE */
...

amitbern · 12-05-2008, 10:11 AM

According to linux coding guidelines:
No ifdefs in .c Code
With the wide number of different processors, different configuration options and variations of the same base hardware types that Linux runs on, it is easy to start having a lot of #ifdef statements in your code. This is not the proper thing to do. Instead, place the #ifdef in a header file, and provide empty inline functions if the code is not to be included.

http://www.linuxjournal.com/article/5780

Again, what i'm trying to do is the following:
Every time i call function1 and RUN_FUNC1 is defined - I want to run function1 which is defined in file.c
Every time i call function1 and RUN_FUNC1 is NOT defined - I want to run the static function1 which is defined in file.h
All of these without using ifdef in the c file...
I'm sure there are people here who know how...

ta0kira · 12-05-2008, 01:44 PM

This isn't in the context of the kernel, but you get the idea.

Code:

//code.h

#include <stdio.h>


extern void function2();


#ifdef RUN_FUNC1

  //(only defined here for illustrative purposes)
  extern void function1() { printf("RUN_FUNC1\n"); }

#else

  void function1() __attribute__ ((alias ("function1_internal")));

#endif

Code:

//code.c

#include "code.h"

#include <stdio.h>

static void function1_internal() { printf("!RUN_FUNC1\n"); }

void function2() { function1(); }

int main() { function2(); }

ta0kira

PS Sorry, missed that the static is defined in the header. But why? What other sources need that definition?

amitbern · 12-06-2008, 02:47 AM

I just can't understand how does this kernel code is compiled...

just look on function hiddev_connect, if CONFIG_USB_HIDDEV is defined, then when calling hiddev_connect - this will call hiddev_connect from hiddev.c
and if CONFIG_USB_HIDDEV is not defined, then when calling hiddev_connect - this will call the inline function from hiddev.h

why didn't this cause the redefinition error in the compilation?...

in include/linux/hiddev.h

Code:

#ifdef CONFIG_USB_HIDDEV
int hiddev_connect(struct hid_device *);
void hiddev_disconnect(struct hid_device *);
void hiddev_hid_event(struct hid_device *hid, struct hid_field *field,
		      struct hid_usage *usage, __s32 value);
void hiddev_report_event(struct hid_device *hid, struct hid_report *report);
int __init hiddev_init(void);
void hiddev_exit(void);
#else
static inline int hiddev_connect(struct hid_device *hid) { return -1; }
static inline void hiddev_disconnect(struct hid_device *hid) { }
static inline void hiddev_hid_event(struct hid_device *hid, struct hid_field *field,
		      struct hid_usage *usage, __s32 value) { }
static inline void hiddev_report_event(struct hid_device *hid, struct hid_report *report) { }
static inline int hiddev_init(void) { return 0; }
static inline void hiddev_exit(void) { }
#endif

in drivers/hid/usbhid/hiddev.c

Code:

int hiddev_connect(struct hid_device *hid)
{
	struct hiddev *hiddev;
	struct usbhid_device *usbhid = hid->driver_data;
	int i;
	int retval;

	for (i = 0; i < hid->maxcollection; i++)
		if (hid->collection[i].type ==
		    HID_COLLECTION_APPLICATION &&
		    !IS_INPUT_APPLICATION(hid->collection[i].usage))
			break;

	if (i == hid->maxcollection && (hid->quirks & HID_QUIRK_HIDDEV) == 0)
		return -1;

	if (!(hiddev = kzalloc(sizeof(struct hiddev), GFP_KERNEL)))
		return -1;

	retval = usb_register_dev(usbhid->intf, &hiddev_class);
	if (retval) {
		err_hid("Not able to get a minor for this device.");
		kfree(hiddev);
		return -1;
	}

	init_waitqueue_head(&hiddev->wait);
	INIT_LIST_HEAD(&hiddev->list);
	spin_lock_init(&hiddev->list_lock);
	hiddev->hid = hid;
	hiddev->exist = 1;

	hid->minor = usbhid->intf->minor;
	hid->hiddev = hiddev;

	hiddev_table[usbhid->intf->minor - HIDDEV_MINOR_BASE] = hiddev;

	return 0;
}

ErV · 12-06-2008, 03:21 AM

Quote:

Originally Posted by amitbern

I just can't understand how does this kernel code is compiled...

just look on function hiddev_connect, if CONFIG_USB_HIDDEV is defined, then when calling hiddev_connect - this will call hiddev_connect from hiddev.c
and if CONFIG_USB_HIDDEV is not defined, then when calling hiddev_connect - this will call the inline function from hiddev.h

why didn't this cause the redefinition error in the compilation?...

Most likely because hidev.c isn't compiled when CONFIG_USB_HIDDEV isn't defined.
drivers/hid/usbhid/Makefile:

Code:

#
# Makefile for the USB input drivers
#

# Multipart objects.
usbhid-objs	:= hid-core.o hid-quirks.o

# Optional parts of multipart objects.

ifeq ($(CONFIG_USB_HIDDEV),y)
	usbhid-objs	+= hiddev.o
endif
ifeq ($(CONFIG_HID_PID),y)
	usbhid-objs	+= hid-pidff.o
endif
ifeq ($(CONFIG_LOGITECH_FF),y)
	usbhid-objs	+= hid-lgff.o
endif
ifeq ($(CONFIG_PANTHERLORD_FF),y)
	usbhid-objs	+= hid-plff.o
endif
ifeq ($(CONFIG_THRUSTMASTER_FF),y)
	usbhid-objs	+= hid-tmff.o
endif
ifeq ($(CONFIG_ZEROPLUS_FF),y)
	usbhid-objs	+= hid-zpff.o
endif
ifeq ($(CONFIG_HID_FF),y)
	usbhid-objs	+= hid-ff.o
endif

obj-$(CONFIG_USB_HID)		+= usbhid.o
obj-$(CONFIG_USB_KBD)		+= usbkbd.o
obj-$(CONFIG_USB_MOUSE)		+= usbmouse.o

Pay close attention to this part:

Code:

ifeq ($(CONFIG_USB_HIDDEV),y)
	usbhid-objs	+= hiddev.o
endif

voyvf · 12-06-2008, 11:01 PM

Quote:

Originally Posted by amitbern

I just can't understand how does this kernel code is compiled...

just look on function hiddev_connect, if CONFIG_USB_HIDDEV is defined, then when calling hiddev_connect - this will call hiddev_connect from hiddev.c
and if CONFIG_USB_HIDDEV is not defined, then when calling hiddev_connect - this will call the inline function from hiddev.h

why didn't this cause the redefinition error in the compilation?..

The "ifdef" portion of the article here might offer some insight as to how it works.

Hope this helps.

ErV · 12-07-2008, 04:27 AM

Quote:

Originally Posted by voyvf

The "ifdef" portion of the article here might offer some insight as to how it works.

Hope this helps.

It's not that easy. He meant that if *.c file will be included in compilation, then people should get "function already defined" (or whatever it is exactly called) error, when there is inline function in header. This doesn't happen because *.c source doesn't get included into compilation when such error could happen.

voyvf · 12-07-2008, 03:34 PM

Quote:

Originally Posted by ErV

It's not that easy. He meant that if *.c file will be included in compilation, then people should get "function already defined" (or whatever it is exactly called) error, when there is inline function in header. This doesn't happen because *.c source doesn't get included into compilation when such error could happen.

Ah, I misunderstood the issue. Sorry about that.