diff mbox

[U-Boot] Function prototype conflicts with standalone apps

Message ID CAFOYHZBYxUr8uBALOUpe-doMdfzK8McM9t=QOp_3N4bPFtaTWw@mail.gmail.com
State RFC
Delegated to: Wolfgang Denk
Headers show

Commit Message

Chris Packham Jan. 16, 2013, 8:01 p.m. UTC 
On Thu, Jan 17, 2013 at 1:57 AM, Albert ARIBAUD
<albert.u.boot@aribaud.net>wrote:

> Hi Chris,
>
> On Wed, 16 Jan 2013 23:16:07 +1300, Chris Packham
> <judge.packham@gmail.com> wrote:
>
> > Hi Albert,
> >
> > On 01/16/2013 08:25 PM, Albert ARIBAUD (U-Boot) wrote:
> > > Hi Chris,
> > >
> > > On Wed, 16 Jan 2013 17:23:58 +1300, Chris Packham
> > > <judge.packham@gmail.com> wrote:
> > >
> > >> Hi,
> > >>
> > >> I've just run into something porting an existing out of tree board to
> > >> u-boot 2012.10 but I think it points to a generic issue for standalone
> > >> applications.
> > >>
> > >> Consider the following change
> > >>
> > >> diff --git a/examples/standalone/hello_world.c
> > >> b/examples/standalone/hello_world.c
> > >> index 067c390..d2e6a77 100644
> > >> --- a/examples/standalone/hello_world.c
> > >> +++ b/examples/standalone/hello_world.c
> > >> @@ -24,7 +24,7 @@
> > >>  #include <common.h>
> > >>  #include <exports.h>
> > >>
> > >> -int hello_world (int argc, char * const argv[])
> > >> +int net_init (int argc, char * const argv[])
> > >>  {
> > >>         int i;
> > >>
> > >> Because I'm not linking with the u-boot object file, I should be able
> to
> > >> use any function name I like in my application as long as it isn't
> one of
> > >> the functions in exports.h (at least in theory). Unfortunately I end
> up
> > >> with the following compiler error
> > >>
> > >>   hello_world.c:27: error: conflicting types for ‘net_init’
> > >>   uboot/include/net.h:489: error: previous declaration of ‘net_init’
> was
> > >> here
> > >>   make[1]: *** [hello_world.o] Error 1
> > >>
> > >> If I replace #include <common.h> in my app with the first hunk of
> includes
> > >> from the top of common.h  then I can compile just fine.
> > >>
> > >> I was wondering if it made sense to people to have standalone
> applications
> > >> define something like __STANDALONE__ either via CPPFLAGS or in the
> source
> > >> itself and use the presence of that to exclude the majority of
> common.h
> > >> when used in standalone applications. Or alternatively move the
> required
> > >> bits to exports.h.
> > >
> > > (long rant ahead. Short answer after end of rant)
> >
> > Short response: Yep I can live with that by making some changes to my
> > standalone application. I just thought it might be cleaner if a minimal
> > set of definitions were provided.
> >
> > > [RANT]
> > >
> > > Code writers indeed have a right to name any function or other object
> > > any way they choose... within the constraints of the situation.
> > >
> > > Some of these constraints stem from the tools -- you just cannot put an
> > > ampersand in a C object name, for instance -- and some stem from the
> > > 'agreement' entered into when using a library -- precisely, the
> > > agreement on the name and semantics of such and such object names.
> > >
> > > Here, by including exports.h, you enter an agreement in which
> > > the object name 'net_init' receives a specific meaning. What you want
> > > is to benefit from the agreement without abiding by it.
> > >
> > > Now this can be changed, technically, as most things are, and a new
> > > kind of agreement could be devised with fine-grain control on which
> > > object names would or would not be defined. The question is, *should*
> > > this be done?
> > >
> > > Would you, analogously, suggest that Linux app developers be able to
> > > opt out of defining fopen() when they #include <stdio.h> because they
> > > feel they have a right to define 'char* fopen(float F)' in their code
> if
> > > they so please? And that it should be done so for just about any
> > > kernel-exported symbol? I suspect not.
> >
> > Actually this is my point. The symbols aren't exported. They're just in
> > the header file. The kernel solution for this is using __KERNEL__ and
> > filtering the exported headers to remove the kernel internals not needed
> > by userland. If for some reason I did define a different fopen I'd get a
> > link error whether I included stdio or not.
>
> Ok, so I misunderstood your objective -- my bad. It is not a problem of
> wanting to use a function which is part of the API; it's a problem of
> having to pull in non-API function declarations along with the API ones.
>
> I still think it's a bad idea to use a homonym of an existing U-Boot
> function, but at least you should not be prevented from doing so if
> the function is not actually in the API.
>
> But also I still dislike the solution you are suggestiong, because I
> think that the actual problem is not properly stated. Re-reading the
> common.h and exports.h header files, IIUC the actual problem is that
> exports.h depends on some types from common.h but common.h also defines
> some internal U-Boot functions. The problem is that common.h is needed
> for different and incompatible purposes: (public) API on the one hand,
> internal declarations on the other hand.
>
> Thus, I'd rather have common.h simply split into "a header file
> with basic types needed by both public API and internal code" and "a
> header file defining some internal U-Boot functions, possibly
> #including the shared basic types header file", with no conditional
> involved. New header file names to be chosen wisely.
>

So perhaps a variation on the patch I sent earlier in this thread. Instead
of moving definitions into export.h creating a new file (stddef.h ?) and
moving the things needed by export.h from common.h.

Something like this

---8<---

From 6cc993be1b6db8734f8c91121675a9a48fb9ea77 Mon Sep 17 00:00:00 2001
From: Chris Packham <judge.packham@gmail.com>
Date: Wed, 16 Jan 2013 17:36:05 +1300
Subject: [RFC/PATCH] common.h: move definitions needed by standalone
programs

Standalone applications need to include export.h which in turn needs
some of the basic types defined by including common.h. Unfortunately
common.h also has declarations for functions not available to standalone
applications. For example if a standalone application defined it's own
networking stack it could only defined "int net_init(void)" because this
is the declaration in net.h which is included in common.h.

To avoid symbol namespace conflicts between standalone applications and
u-boot code add the definitions required in exports.h so standalone
applications do not have to include common.h.
---
 examples/standalone/hello_world.c |    1 -
 include/common.h                  |   13 +----------
 include/exports.h                 |    2 ++
 include/stddef.h                  |   43
+++++++++++++++++++++++++++++++++++++
 4 files changed, 46 insertions(+), 13 deletions(-)
 create mode 100644 include/stddef.h
diff mbox

Patch

diff --git a/examples/standalone/hello_world.c
b/examples/standalone/hello_world.c
index 067c390..91a2deb 100644
--- a/examples/standalone/hello_world.c
+++ b/examples/standalone/hello_world.c
@@ -21,7 +21,6 @@ 
  * MA 02111-1307 USA
  */

-#include <common.h>
 #include <exports.h>

 int hello_world (int argc, char * const argv[])
diff --git a/include/common.h b/include/common.h
index a7fb05e..dca33b8 100644
--- a/include/common.h
+++ b/include/common.h
@@ -29,18 +29,7 @@ 

 #ifndef __ASSEMBLY__        /* put C only stuff in this section */

-typedef unsigned char        uchar;
-typedef volatile unsigned long    vu_long;
-typedef volatile unsigned short vu_short;
-typedef volatile unsigned char    vu_char;
-
-#include <config.h>
-#include <asm-offsets.h>
-#include <linux/bitops.h>
-#include <linux/types.h>
-#include <linux/string.h>
-#include <asm/ptrace.h>
-#include <stdarg.h>
+#include <stddef.h>
 #if defined(CONFIG_PCI) && (defined(CONFIG_4xx) && !defined(CONFIG_AP1000))
 #include <pci.h>
 #endif
diff --git a/include/exports.h b/include/exports.h
index 63aa4b2..d874235 100644
--- a/include/exports.h
+++ b/include/exports.h
@@ -3,6 +3,8 @@ 

 #ifndef __ASSEMBLY__

+#include <stddef.h>
+
 /* These are declarations of exported functions available in C code */
 unsigned long get_version(void);
 int  getc(void);
diff --git a/include/stddef.h b/include/stddef.h
new file mode 100644
index 0000000..7fea85a
--- /dev/null
+++ b/include/stddef.h
@@ -0,0 +1,43 @@ 
+/*
+ * (C) Copyright 2000-2009
+ * Wolfgang Denk, DENX Software Engineering, wd@denx.de.
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.     See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+#ifndef __STDDEF_H__
+#define __STDDEF_H__
+
+#ifndef __ASSEMBLY__
+
+typedef unsigned char           uchar;
+typedef volatile unsigned long  vu_long;
+typedef volatile unsigned short vu_short;
+typedef volatile unsigned char  vu_char;
+
+#include <config.h>
+#include <asm-offsets.h>
+#include <linux/bitops.h>
+#include <linux/types.h>
+#include <linux/string.h>
+#include <asm/ptrace.h>
+#include <stdarg.h>
+
+#endif /* __ASSEMBLY__ */
+
+#endif /* __STDDEF_H__ */