All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Robert Schiele <rschiele@gmail.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH] add definitions for global variables to shell.c
Date: Tue, 19 Aug 2008 00:53:21 -0700	[thread overview]
Message-ID: <7vpro5fnke.fsf@gitster.siamese.dyndns.org> (raw)
In-Reply-To: <20080819072650.GE11842@schiele.dyndns.org> (Robert Schiele's message of "Tue, 19 Aug 2008 09:26:51 +0200")

Robert Schiele <rschiele@gmail.com> writes:

> On Mon, Aug 18, 2008 at 05:29:11PM -0700, Junio C Hamano wrote:
>> Haven't looked at the real declarations but if the decl are "extern" and
>> nobody refers to them, why should the resulting object file require them
>> to be defined anywhere?  If the decl are not and in (fortran-ish) "common"
>> section, on the other hand, you shouldn't have to define them yourself
>> like this either.
>> 
>> This sounds like a compiler bug to me.
>
> This was my first thought as well but after more inspection there are two
> things to consider:
>
> 1. I was not really precise enough in my description since I didn't spot that
>    when I looked into the issue first: Actually there are references to these
>    variables in static inline functions in cache.h.  Thus there actually is a
>    reference though one that will never be used since abspath.c (that includes
>    cache.h) is not calling any of these functions.
>
> 2. Since these symbols turn out to be referenced though in dead code only I
>    wouldn't call it a compiler bug.

Ok, as I said, I didn't look.  If they are indeed referenced, that is a
different story.

Even if that is the case, I do not like the prospect of having to maintain
a set of duplicated variable definitions.  If we really wanted to address
this issue, maybe we would want a separate source file that is linked to
both git-shell and to the rest of the system that has nothing but
definitions of these variables?  I thought environment.c was meant to be
something like that -- would linking environment.o pull in too many extra
references these days (again, I didn't try)?

  reply	other threads:[~2008-08-19  7:54 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-08-18 12:37 [PATCH] add definitions for global variables to shell.c Robert Schiele
2008-08-19  0:29 ` Junio C Hamano
2008-08-19  7:26   ` Robert Schiele
2008-08-19  7:53     ` Junio C Hamano [this message]
2008-08-19  8:16       ` Robert Schiele
2008-08-19  8:49       ` Johannes Sixt
2008-08-19  9:18         ` Robert Schiele
2008-08-19 23:09           ` Junio C Hamano
2008-08-20  1:06             ` [PATCH 1/2] shell: do not play duplicated definition games to shrink the executable Junio C Hamano
2008-08-20  1:06             ` [PATCH 2/2] Build-in "git-shell" Junio C Hamano
2008-08-20  6:54               ` Johannes Sixt
2008-08-20  1:06             ` [PATCH] add definitions for global variables to shell.c Junio C Hamano
2008-08-20  4:36               ` Robert Schiele

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=7vpro5fnke.fsf@gitster.siamese.dyndns.org \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=rschiele@gmail.com \
    /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.