From patchwork Wed Jan 16 20:01:46 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Chris Packham X-Patchwork-Id: 213049 X-Patchwork-Delegate: wd@denx.de Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from theia.denx.de (theia.denx.de [85.214.87.163]) by ozlabs.org (Postfix) with ESMTP id 25F302C0079 for ; Thu, 17 Jan 2013 07:02:01 +1100 (EST) Received: from localhost (localhost [127.0.0.1]) by theia.denx.de (Postfix) with ESMTP id 3FBB84A088; Wed, 16 Jan 2013 21:01:58 +0100 (CET) X-Virus-Scanned: Debian amavisd-new at theia.denx.de Received: from theia.denx.de ([127.0.0.1]) by localhost (theia.denx.de [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id BQR8jIQ1Wm96; Wed, 16 Jan 2013 21:01:57 +0100 (CET) Received: from theia.denx.de (localhost [127.0.0.1]) by theia.denx.de (Postfix) with ESMTP id 452D44A08B; Wed, 16 Jan 2013 21:01:55 +0100 (CET) Received: from localhost (localhost [127.0.0.1]) by theia.denx.de (Postfix) with ESMTP id 3BA9B4A08B for ; Wed, 16 Jan 2013 21:01:52 +0100 (CET) X-Virus-Scanned: Debian amavisd-new at theia.denx.de Received: from theia.denx.de ([127.0.0.1]) by localhost (theia.denx.de [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id MZYkdE48Piqy for ; Wed, 16 Jan 2013 21:01:50 +0100 (CET) X-policyd-weight: NOT_IN_SBL_XBL_SPAMHAUS=-1.5 NOT_IN_SPAMCOP=-1.5 NOT_IN_BL_NJABL=-1.5 (only DNSBL check requested) Received: from mail-oa0-f42.google.com (mail-oa0-f42.google.com [209.85.219.42]) by theia.denx.de (Postfix) with ESMTPS id B917C4A088 for ; Wed, 16 Jan 2013 21:01:47 +0100 (CET) Received: by mail-oa0-f42.google.com with SMTP id j1so1868063oag.1 for ; Wed, 16 Jan 2013 12:01:46 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:x-received:in-reply-to:references:date:message-id :subject:from:to:cc:content-type; bh=Zum4moRxolIlHsoqZ1TcBb8zcLBjN3IrVYmMp7YP7MM=; b=ats2N64Vzu0+SWdeHLvwXSDt2rrUYVYmv5uuR18jvcAqgO6bpKmQAblkKs/aXqsGUv 3hl6uz1CxARr8QaHvNHe9n7O/p1FuKp0A8wBffLyn18SX7HAi+d2S5HuFzvs/G/k/aMg hl/3qMUB166NzsIk+VjcodRPPhaRH7fVCcdTdei+2HIISSe1NvKhyQGGCBtt2UiDBjd1 lN4Ff/8NVg6sRozO+4zAbPlpFHcbQofPJhGrt3ZE6g0cckm/WOuLydp01YkJs+VJ8Qd1 bpDd2dezDcZN6t6dEqetlugLaoBGItTScmnWHkjkY1XpB6YENQQk4vLrvsz3Q4NK86mC ZUtg== MIME-Version: 1.0 X-Received: by 10.60.28.74 with SMTP id z10mr1922153oeg.29.1358366506318; Wed, 16 Jan 2013 12:01:46 -0800 (PST) Received: by 10.76.68.38 with HTTP; Wed, 16 Jan 2013 12:01:46 -0800 (PST) In-Reply-To: <20130116135732.6772f905@lilith> References: <20130116082549.4babe813@lilith> <50F67DE7.9050107@gmail.com> <20130116135732.6772f905@lilith> Date: Thu, 17 Jan 2013 09:01:46 +1300 Message-ID: From: Chris Packham To: Albert ARIBAUD X-Content-Filtered-By: Mailman/MimeDel 2.1.11 Cc: U-Boot Subject: Re: [U-Boot] Function prototype conflicts with standalone apps X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.11 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: u-boot-bounces@lists.denx.de Errors-To: u-boot-bounces@lists.denx.de On Thu, Jan 17, 2013 at 1:57 AM, Albert ARIBAUD wrote: > Hi Chris, > > On Wed, 16 Jan 2013 23:16:07 +1300, Chris Packham > 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 > > > 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 > > >> #include > > >> > > >> -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 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 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 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 --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 #include 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 -#include -#include -#include -#include -#include -#include +#include #if defined(CONFIG_PCI) && (defined(CONFIG_4xx) && !defined(CONFIG_AP1000)) #include #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 + /* 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 +#include +#include +#include +#include +#include +#include + +#endif /* __ASSEMBLY__ */ + +#endif /* __STDDEF_H__ */