Remove PRI_MACROS_BROKEN define usage
Commit Message
On 20-03-2014 15:51, Joseph S. Myers wrote:
> On Thu, 20 Mar 2014, Adhemerval Zanella wrote:
>
>> Do we still want to keep code that no longer makes sense in our context?
>> GLIBC does not even have configure check for PRI_MACROS_BROKEN, so
>> should we keep aiming a possible sync? I personally aim for simplicity
>> and maintainability in this case, which means get rid of legacy code
>> that does not make sense for GLIBC.
> We want all files shared with other projects to be identical with the
> copies in those projects as far as possible (there may sometimes be cases
> where license notices need to be different, as with the files from GMP),
> to facilitate merging in both directions. This includes files shared with
> gnulib and gettext.
>
> (This does not necessarily mean a particular conditional is still relevant
> to the other projects; they too may remove support for some older systems,
> although supporting a much wider range of systems than glibc. But you'd
> need to consult with the gettext and gnulib communities to determine
> whether a conditional is still relevant there.)
>
Ok, I check with gettext implementation 'gettext-runtime/intl/loadmsgcat.c' and changed
the initial patch based on your suggestion (I also added some changes to make
glibc one similar to gettext one).
---
Comments
On Thu, 20 Mar 2014, Adhemerval Zanella wrote:
> Ok, I check with gettext implementation
> 'gettext-runtime/intl/loadmsgcat.c' and changed the initial patch based
> on your suggestion (I also added some changes to make glibc one similar
> to gettext one).
Any change brought from gettext would best be committed on its own
(ideally with the original author of that change in gettext being
credited).
On 20-03-2014 21:01, Joseph S. Myers wrote:
> On Thu, 20 Mar 2014, Adhemerval Zanella wrote:
>
>> Ok, I check with gettext implementation
>> 'gettext-runtime/intl/loadmsgcat.c' and changed the initial patch based
>> on your suggestion (I also added some changes to make glibc one similar
>> to gettext one).
> Any change brought from gettext would best be committed on its own
> (ideally with the original author of that change in gettext being
> credited).
>
The changes I have incorporated on the patch is just change the old K&R
function prototype to ISO C, remove and superfluous ';' and change a
free call to remove an unnecessary cast. For these straightforward patches
do we really need to track the ones how made then and commit a set of
patches?
On Thu, 20 Mar 2014, Adhemerval Zanella wrote:
> > Any change brought from gettext would best be committed on its own
> > (ideally with the original author of that change in gettext being
> > credited).
> >
> The changes I have incorporated on the patch is just change the old K&R
> function prototype to ISO C, remove and superfluous ';' and change a
> free call to remove an unnecessary cast. For these straightforward patches
> do we really need to track the ones how made then and commit a set of
> patches?
You could probably just say "Change merged from gettext." or similar
without detailed attribution - but a change coming from gettext should
still be committed separately from a change not coming from gettext.
On 03/20/2014 03:20 PM, Adhemerval Zanella wrote:
> diff --git a/intl/loadmsgcat.c b/intl/loadmsgcat.c
> index b96a997..d47b903 100644
> --- a/intl/loadmsgcat.c
> +++ b/intl/loadmsgcat.c
> @@ -62,6 +62,7 @@ char *alloca ();
> #ifdef _LIBC
> # include <langinfo.h>
> # include <locale.h>
> +# define PRI_MACROS_BROKEN 0
> #endif
>
> #if (defined HAVE_MMAP && defined HAVE_MUNMAP && !defined DISALLOW_MMAP) \
This hunk could go in immediately to fix the -Wundef issues while
take our time to merge a new gettext update.
I was looking at this today, but it's going to be more work than
I have free time for this week.
Even though Steve Ellcey has gotten upstream to change the code
such that PRI_MACROS_BROKEN is always defined, there is no stable
release with that change yet.
Cheers,
Carlos.
On Fri, 2 May 2014, Carlos O'Donell wrote:
> Even though Steve Ellcey has gotten upstream to change the code
> such that PRI_MACROS_BROKEN is always defined, there is no stable
> release with that change yet.
I don't think gettext releases are relevant to us; it's the development
sources of glibc's libintl that we should aim to have in sync with the
development sources of gettext's libintl, by merging local changes in both
directions and implementing things in such a way that identical code can
work in both places.
On 05/02/2014 11:12 AM, Joseph S. Myers wrote:
> On Fri, 2 May 2014, Carlos O'Donell wrote:
>
>> Even though Steve Ellcey has gotten upstream to change the code
>> such that PRI_MACROS_BROKEN is always defined, there is no stable
>> release with that change yet.
>
> I don't think gettext releases are relevant to us; it's the development
> sources of glibc's libintl that we should aim to have in sync with the
> development sources of gettext's libintl, by merging local changes in both
> directions and implementing things in such a way that identical code can
> work in both places.
May I suggest the following plan of action then?
* Merge libintl 0.18.3.2 (official release) into glibc and fixup
where appropriate, testing the result.
* Pro: You know libitinl works because it's a released version.
* Con: Subsequent merge from glibc to gettext is harder.
* Review remaining differences between glibc master and gettext
master and propose patches.
This reduces the merge-in risk of breaking glibc by using
a stable gettext, but allows us to iterate down to zero
differences.
The 0.18.3.2 differences aren't large, but they still have
new generic lock thread support, and pathname support, and
merging this in worries me slightly. That worry is why I
recommend a conservative first step.
Are you suggesting that this is really just a waste of time
given libintl's stability and doing the above is just 2x
the work for no reward?
Cheers,
Carlos.
On Fri, 2 May 2014, Carlos O'Donell wrote:
> The 0.18.3.2 differences aren't large, but they still have
> new generic lock thread support, and pathname support, and
> merging this in worries me slightly. That worry is why I
> recommend a conservative first step.
If concerned about any changes, then splitting up the merge makes sense -
but splitting it up on the basis of *separate logical changes* rather than
using a particular libintl release. (Splitting up matters less for
changes that are completely routine cleanups, though such cleanups
shouldn't be mixed with substantive changes.)
> I don't think gettext releases are relevant to us; it's the development
> sources of glibc's libintl that we should aim to have in sync with the
> development sources of gettext's libintl, by merging local changes in both
> directions and implementing things in such a way that identical code can
> work in both places.
Agreed. We don't worry about releasedness for any of the code we share
with other GNU projects. We vet the changes like any other changes and
rely on our own vetting and our own testing. Of course, for any shared
file whose source of truth changes more often than once in a blue moon,
it's wise to pay attention to the owning project and make sure we've kept
up with all its changes when either that project or libc is nearing release.
On 05/02/2014 03:27 PM, Roland McGrath wrote:
>> I don't think gettext releases are relevant to us; it's the development
>> sources of glibc's libintl that we should aim to have in sync with the
>> development sources of gettext's libintl, by merging local changes in both
>> directions and implementing things in such a way that identical code can
>> work in both places.
>
> Agreed. We don't worry about releasedness for any of the code we share
> with other GNU projects. We vet the changes like any other changes and
> rely on our own vetting and our own testing. Of course, for any shared
> file whose source of truth changes more often than once in a blue moon,
> it's wise to pay attention to the owning project and make sure we've kept
> up with all its changes when either that project or libc is nearing release.
I'll look at gettext master then. I appreciate you and Joseph commenting
promptly before I wasted my time with an intermediate merge if nobody
thought that was a good idea.
c.
@@ -62,6 +62,7 @@ char *alloca ();
#ifdef _LIBC
# include <langinfo.h>
# include <locale.h>
+# define PRI_MACROS_BROKEN 0
#endif
#if (defined HAVE_MMAP && defined HAVE_MUNMAP && !defined DISALLOW_MMAP) \
@@ -486,8 +487,7 @@ int _nl_msg_cat_cntr;
/* Expand a system dependent string segment. Return NULL if unsupported. */
static const char *
-get_sysdep_segment_value (name)
- const char *name;
+get_sysdep_segment_value (const char *name)
{
/* Test for an ISO C 99 section 7.8.1 format string directive.
Syntax:
@@ -756,9 +756,8 @@ get_sysdep_segment_value (name)
message catalog do nothing. */
void
internal_function
-_nl_load_domain (domain_file, domainbinding)
- struct loaded_l10nfile *domain_file;
- struct binding *domainbinding;
+_nl_load_domain (struct loaded_l10nfile *domain_file,
+ struct binding *domainbinding)
{
__libc_lock_define_initialized_recursive (static, lock);
int fd = -1;
@@ -821,7 +820,7 @@ _nl_load_domain (domain_file, domainbinding)
|| __builtin_expect ((size = (size_t) st.st_size) != st.st_size, 0)
|| __builtin_expect (size < sizeof (struct mo_file_header), 0))
/* Something went wrong. */
- goto out;;
+ goto out;
#ifdef HAVE_MMAP
/* Now we are ready to load the file. If mmap() is available we try
@@ -1277,8 +1276,7 @@ _nl_load_domain (domain_file, domainbinding)
#ifdef _LIBC
void
internal_function __libc_freeres_fn_section
-_nl_unload_domain (domain)
- struct loaded_domain *domain;
+_nl_unload_domain (struct loaded_domain *domain)
{
size_t i;
@@ -1289,7 +1287,7 @@ _nl_unload_domain (domain)
{
struct converted_domain *convd = &domain->conversions[i];
- free ((char *) convd->encoding);
+ free (convd->encoding);
if (convd->conv_tab != NULL && convd->conv_tab != (char **) -1)
free (convd->conv_tab);
if (convd->conv != (__gconv_t) -1)