All of lore.kernel.org
 help / color / mirror / Atom feed
From: Russell King <rmk@arm.linux.org.uk>
To: Sam Ravnborg <sam@ravnborg.org>
Cc: linux-arch@vger.kernel.org, Sam Ravnborg <sam@mars.ravnborg.org>
Subject: Re: [RFC PATCH] get rid of duplicate exports from string.h
Date: Sat, 11 Mar 2006 17:51:01 +0000	[thread overview]
Message-ID: <20060311175101.GA31096@flint.arm.linux.org.uk> (raw)
In-Reply-To: <20060311161106.GA19455@mars.ravnborg.org>

On Sat, Mar 11, 2006 at 05:11:06PM +0100, Sam Ravnborg wrote:
> In lib/string.c export all symbols that are not defines.
>...
> As an example I've done it for sparc64 but the same applies
> for most architectures.

A few comments:

1. it'd be nice to use #ifndef blah instead of #if !defined(blah)

2. I don't think we normally indent for preprocessor conditionals -
   if we did, would we not have these function definitions indented?

3. I don't understand how these !defined things are supposed to solve
   the problem.  There are three cases, and use strcpy as an example:

   a) an architecture decides to override the lib/string.c definition
      with its own static inline function.  In this case, it must
      define __HAVE_ARCH_STRCPY to exclude the lib/string.c version.

      In this case, the architecture may decide against exporting the
      named function because it'll always be inlined.

   b) an architecture decides to override the lib/string.c definition
      with its own macro version.  This case is essentially the same
      as case (a).

   c) an architecture decides to override the lib/string.c definition
      with its own out of line version.  It must define
      __HAVE_ARCH_STRCPY as per (a), but it wants an export.

   d) an architecture wishes to use the lib/string.c definition.  It
      must not define __HAVE_ARCH_STRCPY.

   With these changes, case (a) requires an additional #define strcpy strcpy
   to prevent the EXPORT_SYMBOL being used.

   Since we have case (a) in the kernel sources, I don't see any additional
   #defines being added to prevent these exports.  Maybe you decided to
   always cause lib/string.c to export them except for case (b) ?

All in all, I think this is a complex solution to something which should
be simple.  We have the general rule that EXPORT_SYMBOLs should be local
to the code which provides the function - at least within the same file.
This change seems to be a step away from that.

I much prefer the current implementation where, if you define __HAVE_ARCH_foo
then foo is not defined nor exported by the core kernel.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 Serial core

  reply	other threads:[~2006-03-11 17:51 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-03-11 16:11 [RFC PATCH] get rid of duplicate exports from string.h Sam Ravnborg
2006-03-11 17:51 ` Russell King [this message]
2006-03-11 18:27   ` Sam Ravnborg

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20060311175101.GA31096@flint.arm.linux.org.uk \
    --to=rmk@arm.linux.org.uk \
    --cc=linux-arch@vger.kernel.org \
    --cc=sam@mars.ravnborg.org \
    --cc=sam@ravnborg.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.