diff mbox

Breakage with [PATCH, libgfortran] PR60324 Handle arbitrarily long path names

Message ID alpine.BSF.2.02.1405221846090.39961@arjuna.pair.com
State New
Headers show

Commit Message

Hans-Peter Nilsson May 23, 2014, 2:23 a.m. UTC 
On Fri, 23 May 2014, Janne Blomqvist wrote:
> On Thu, May 22, 2014 at 6:36 PM, Hans-Peter Nilsson <hp@bitrange.com> wrote:
> > This patch broke build for newlib targets; you need AC_DEFINE
> > clauses for those in the "if-then"-leg where you patched the
> > "else"-leg.
>
> Do I?

The way that configure-clause is written, that's obviously the
intended means.

> The patch does include fallback implementations for strnlen and
> strndup in case the target doesn't supply them.

The target *does* supply them, but the configuration machinery
doesn't (can't) look, as linking may not (yet) be possible.

> I don't know whether
> newlib does, or if current versions do have it but old still in use
> versions don't etc.

Both are in newlib since 2002 (two years before the release of
the required minimum host-gcc-version), no need to add such
complexity.

> I suggest that whoever was insisting on this
> stupid hard-coded func list for newlib takes care of figuring that out
> and, if it turns out that all relevant newlib versions do supply one
> or both of those functions, posts a patch.

I'm not defending the existing solution, I was observing your
patch breaking it.  The obvious fix is adjustments by means of
this existing machinery; done.  I suggest breakages be fixed
without shooting messengers or requiring jumping through even
stupider hoops in order to fix an obvious immediate breakage.

If you take issue with that machinery, that's a separate issue
which shouldn't be holding up fixing a breakage.

> Or hmm, is there some symbol name conflict, in case we try to use a
> fallback implementation but the target already defines it?

Exactly.  The declaration was clashing with that included from
string.h.

Committed as obvious knowing the cause and the effect and
the prerequisites, after observing build passing the point of
failure for cris-elf.

	* configure.ac [with_newlib] (HAVE_STRNLEN, HAVE_STRNDUP): Define.
	* configure: Regenerate.


brgds, H-P

Comments

Janne Blomqvist May 26, 2014, 10:04 a.m. UTC | #1 
On Fri, May 23, 2014 at 5:23 AM, Hans-Peter Nilsson <hp@bitrange.com> wrote:
> I'm not defending the existing solution, I was observing your
> patch breaking it.  The obvious fix is adjustments by means of
> this existing machinery; done.  I suggest breakages be fixed
> without shooting messengers or requiring jumping through even
> stupider hoops in order to fix an obvious immediate breakage.
>
> If you take issue with that machinery, that's a separate issue
> which shouldn't be holding up fixing a breakage.

Sorry, I didn't mean to imply that any of this is your fault, on the
contrary I'm happy to have prompt reports from targets I don't use
myself. Thanks for fixing it!
diff mbox

Patch

Index: libgfortran/configure.ac
===================================================================
--- libgfortran/configure.ac	(revision 210821)
+++ libgfortran/configure.ac	(working copy)
@@ -273,6 +273,8 @@  if test "x${with_newlib}" = "xyes"; then
    AC_DEFINE(HAVE_VSNPRINTF, 1, [Define if you have vsnprintf.])
    AC_DEFINE(HAVE_LOCALTIME_R, 1, [Define if you have localtime_r.])
    AC_DEFINE(HAVE_GMTIME_R, 1, [Define if you have gmtime_r.])
+   AC_DEFINE(HAVE_STRNLEN, 1, [Define if you have strnlen.])
+   AC_DEFINE(HAVE_STRNDUP, 1, [Define if you have strndup.])

    # At some point, we should differentiate between architectures
    # like x86, which have long double versions, and alpha/powerpc/etc.,