[RFC,v6,06/16] Provide backward compatibility for strftime family (bug 10871).

Submitted by Rafal Luzynski on March 20, 2017, 8:39 p.m.

Details

Message ID 1999737291.88189.1490042366732@poczta.nazwa.pl
State New
Headers show

Commit Message

Rafal Luzynski March 20, 2017, 8:39 p.m.
As %OB format specifier has been added to strftime/wcsftime
family of functions backward compatibility implementation must be
provided for older binaries which assume that %B returns
a month name in the nominative case.

[BZ #10871]
* include/time.h: Declare __strftime_l_common.
* include/wchar.h: Declare __wcsftime_l_common.
* time/Versions (libc: GLIBC_2.26):
  New (__)strftime(_l) and (__)wcsftime(_l) added.
* time/strftime.c: Provide backward compatible version.
* time/strftime_l.c: Likewise.
* time/wcsftime.c: Likewise.
* time/wcsftime_l.c: Likewise.
---
 include/time.h    |  9 +++++++++
 include/wchar.h   |  9 +++++++++
 time/Versions     |  4 ++++
 time/strftime.c   | 20 +++++++++++++++++---
 time/strftime_l.c | 56 +++++++++++++++++++++++++++++++++++++++++++++----------
 time/wcsftime.c   | 22 ++++++++++++++++++----
 time/wcsftime_l.c | 26 +++++++++++++++++++++++++-
 7 files changed, 128 insertions(+), 18 deletions(-)

Comments

Rafal Luzynski April 7, 2017, 11:01 p.m.
Ping! I have got no feedback so I'll comment my own code:

20.03.2017 21:39 Rafal Luzynski <digitalfreak@lingonborough.com> wrote:
>
> [...]
> @@ -446,7 +453,7 @@ static size_t __strftime_internal (CHAR_T *, size_t, const
> CHAR_T *,
>
> size_t
> my_strftime (CHAR_T *s, size_t maxsize, const CHAR_T *format,
> - const struct tm *tp ut_argument_spec LOCALE_PARAM)
> + const struct tm *tp ut_argument_spec FEATURE_OB_PARAM LOCALE_PARAM)
> {

I really don't like this hack with introducing this additional
FEATURE_OB_PARAM which may be defined to something useful or defined
to an empty string. This introduces lots of clutter to this code.
It's bad especially since the changes from glibc is occasionally applied
to gnulib, a library providing GNU features to the applications being
ported to non-GNU systems but also used by some Linux programs, for
example the date command line utility.

It would be easier if I implemented some compat_strftime() function
which would translate the format specifier so it produces the same
results as the old my_strftime() implementation and pass it to the new
my_strftime(). This would mean translating all %B to %OB and all %OB
to %%OB, of course maintaining the modifiers. But it would have to
allocate memory for the translated format specifier and I don't know
if you like allocating and freeing memory inside glibc.

> [...]
> +strong_alias (__strftime_l_internal, __strftime_l_internal_alias)
> +versioned_symbol (libc, __strftime_l_internal_alias,
> + __strftime_l, GLIBC_2_26);
> +libc_hidden_ver (__strftime_l_internal_alias, __strftime_l)
> +versioned_symbol (libc, __strftime_l_internal, strftime_l, GLIBC_2_26);
> +libc_hidden_ver (__strftime_l_internal, strftime_l)
> +
> +# if SHLIB_COMPAT (libc, GLIBC_2_3, GLIBC_2_26)
> +size_t
> +attribute_compat_text_section
> +__strftime_l_compat (char *s, size_t maxsize, const char *format,
> + const struct tm *tp, __locale_t loc)
> +{
> + return my_strftime (s, maxsize, format, tp, 0, loc);
> +}
> +strong_alias (__strftime_l_compat, __strftime_l_compat_alias)
> +compat_symbol (libc, __strftime_l_compat_alias, __strftime_l, GLIBC_2_3);
> +compat_symbol (libc, __strftime_l_compat, strftime_l, GLIBC_2_3);
> +# endif

Another dirty way to provide two versioned symbols: strftime_l
and __strftime_l pointing to the same internal function. Any tips
how to do it more correctly without introducing another alias?

Regards,

Rafal

Patch hide | download patch | download mbox

diff --git a/include/time.h b/include/time.h
index 6badf0e..8606485 100644
--- a/include/time.h
+++ b/include/time.h
@@ -10,6 +10,15 @@  extern __typeof (strftime_l) __strftime_l;
 libc_hidden_proto (__strftime_l)
 extern __typeof (strptime_l) __strptime_l;
 
+/* Backward compatibility function: feature_OB argument specifies
+   whether or not %OB format specifier should be implemented.  */
+extern size_t __strftime_l_common (char *__restrict __s, size_t __maxsize,
+				   const char *__restrict __format,
+				   const struct tm *__restrict __tp,
+				   const int feature_OB,
+				   __locale_t __loc) __THROW;
+libc_hidden_proto (__strftime_l_common)
+
 libc_hidden_proto (time)
 libc_hidden_proto (asctime)
 libc_hidden_proto (mktime)
diff --git a/include/wchar.h b/include/wchar.h
index 6272130..495ad91 100644
--- a/include/wchar.h
+++ b/include/wchar.h
@@ -25,6 +25,15 @@  libc_hidden_proto (__wcstof_l)
 libc_hidden_proto (__wcstold_l)
 libc_hidden_proto (__wcsftime_l)
 
+/* Backward compatibility function: feature_OB argument specifies
+   whether or not %OB format specifier should be implemented.  */
+extern size_t __wcsftime_l_common (wchar_t *__restrict __s, size_t __maxsize,
+				   const wchar_t *__restrict __format,
+				   const struct tm *__restrict __tp,
+				   const int feature_OB,
+				   __locale_t __loc) __THROW;
+libc_hidden_proto (__wcsftime_l_common)
+
 
 extern double __wcstod_internal (const wchar_t *__restrict __nptr,
 				 wchar_t **__restrict __endptr, int __group)
diff --git a/time/Versions b/time/Versions
index fd83818..da38335 100644
--- a/time/Versions
+++ b/time/Versions
@@ -65,4 +65,8 @@  libc {
   GLIBC_2.16 {
     timespec_get;
   }
+  GLIBC_2.26 {
+    __strftime_l; __wcsftime_l;
+    strftime; strftime_l; wcsftime; wcsftime_l;
+  }
 }
diff --git a/time/strftime.c b/time/strftime.c
index eeab20e..709046e 100644
--- a/time/strftime.c
+++ b/time/strftime.c
@@ -17,11 +17,25 @@ 
 
 #include <time.h>
 #include <locale/localeinfo.h>
+#include <shlib-compat.h>
 
 
 size_t
-strftime (char *s, size_t maxsize, const char *format, const struct tm *tp)
+__strftime (char *s, size_t maxsize, const char *format, const struct tm *tp)
 {
-  return __strftime_l (s, maxsize, format, tp, _NL_CURRENT_LOCALE);
+  return __strftime_l_common (s, maxsize, format, tp, 1, _NL_CURRENT_LOCALE);
 }
-libc_hidden_def (strftime)
+versioned_symbol (libc, __strftime, strftime, GLIBC_2_26);
+libc_hidden_ver (__strftime, strftime)
+
+
+#if SHLIB_COMPAT (libc, GLIBC_2_0, GLIBC_2_26)
+size_t
+attribute_compat_text_section
+__strftime_compat (char *s, size_t maxsize, const char *format,
+		   const struct tm *tp)
+{
+  return __strftime_l_common (s, maxsize, format, tp, 0, _NL_CURRENT_LOCALE);
+}
+compat_symbol (libc, __strftime_compat, strftime, GLIBC_2_0);
+#endif
diff --git a/time/strftime_l.c b/time/strftime_l.c
index 544c97d..7d09250 100644
--- a/time/strftime_l.c
+++ b/time/strftime_l.c
@@ -56,6 +56,8 @@ 
 extern char *tzname[];
 #endif
 
+#include <shlib-compat.h>
+
 /* Do multibyte processing if multibytes are supported, unless
    multibyte sequences are safe in formats.  Multibyte sequences are
    safe if they cannot contain byte sequences that look like format
@@ -279,15 +281,19 @@  static const CHAR_T zeroes[16] = /* "0000000000000000" */
    function gets as an additional argument the locale which has to be
    used.  To access the values we have to redefine the _NL_CURRENT
    macro.  */
-# define strftime		__strftime_l
-# define wcsftime		__wcsftime_l
+# define strftime		__strftime_l_common
+# define wcsftime		__wcsftime_l_common
 # undef _NL_CURRENT
 # define _NL_CURRENT(category, item) \
   (current->values[_NL_ITEM_INDEX (item)].string)
+# define FEATURE_OB_PARAM , int feature_OB
+# define FEATURE_OB_ARG , feature_OB
 # define LOCALE_PARAM , __locale_t loc
 # define LOCALE_ARG , loc
 # define HELPER_LOCALE_ARG  , current
 #else
+# define FEATURE_OB_PARAM
+# define FEATURE_OB_ARG
 # define LOCALE_PARAM
 # define LOCALE_ARG
 # ifdef _LIBC
@@ -435,6 +441,7 @@  static CHAR_T const month_name[][10] =
 static size_t __strftime_internal (CHAR_T *, size_t, const CHAR_T *,
 				   const struct tm *, bool *
 				   ut_argument_spec
+				   FEATURE_OB_PARAM
 				   LOCALE_PARAM) __THROW;
 
 /* Write information from TP into S according to the format
@@ -446,7 +453,7 @@  static size_t __strftime_internal (CHAR_T *, size_t, const
CHAR_T *,
 
 size_t
 my_strftime (CHAR_T *s, size_t maxsize, const CHAR_T *format,
-	     const struct tm *tp ut_argument_spec LOCALE_PARAM)
+	     const struct tm *tp ut_argument_spec FEATURE_OB_PARAM LOCALE_PARAM)
 {
 #if !defined _LIBC && HAVE_TZNAME && HAVE_TZSET
   /* Solaris 2.5 tzset sometimes modifies the storage returned by localtime.
@@ -457,7 +464,7 @@  my_strftime (CHAR_T *s, size_t maxsize, const CHAR_T
*format,
 #endif
   bool tzset_called = false;
   return __strftime_internal (s, maxsize, format, tp, &tzset_called
-			      ut_argument LOCALE_ARG);
+			      ut_argument FEATURE_OB_ARG LOCALE_ARG);
 }
 #ifdef _LIBC
 libc_hidden_def (my_strftime)
@@ -466,10 +473,12 @@  libc_hidden_def (my_strftime)
 static size_t
 __strftime_internal (CHAR_T *s, size_t maxsize, const CHAR_T *format,
 		     const struct tm *tp, bool *tzset_called
-		     ut_argument_spec LOCALE_PARAM)
+		     ut_argument_spec FEATURE_OB_PARAM LOCALE_PARAM)
 {
 #if defined _LIBC && defined USE_IN_EXTENDED_LOCALE_MODEL
   struct __locale_data *const current = loc->__locales[LC_TIME];
+#else
+# define feature_OB 1
 #endif
 
   int hour12 = tp->tm_hour;
@@ -781,13 +790,15 @@  __strftime_internal (CHAR_T *s, size_t maxsize, const
CHAR_T *format,
 	case L_('B'):
 	  if (modifier == L_('E'))
 	    goto bad_format;
+	  if (!feature_OB && modifier == L_('O'))
+	    goto bad_format;
 	  if (change_case)
 	    {
 	      to_uppcase = 1;
 	      to_lowcase = 0;
 	    }
 #if defined _NL_CURRENT || !HAVE_STRFTIME
-	  if (modifier == L_('O'))
+	  if (!feature_OB || modifier == L_('O'))
 	    cpy (STRLEN (f_altmonth), f_altmonth);
 	  else
 	    cpy (STRLEN (f_month), f_month);
@@ -819,10 +830,10 @@  __strftime_internal (CHAR_T *s, size_t maxsize, const
CHAR_T *format,
 	    CHAR_T *old_start = p;
 	    size_t len = __strftime_internal (NULL, (size_t) -1, subfmt,
 					      tp, tzset_called ut_argument
-					      LOCALE_ARG);
+					      FEATURE_OB_ARG LOCALE_ARG);
 	    add (len, __strftime_internal (p, maxsize - i, subfmt,
 					   tp, tzset_called ut_argument
-					   LOCALE_ARG));
+					   FEATURE_OB_ARG LOCALE_ARG));
 
 	    if (to_uppcase)
 	      while (old_start < p)
@@ -1423,10 +1434,35 @@  size_t
 emacs_strftime (char *s, size_t maxsize, const char *format,
 		const struct tm *tp)
 {
-  return my_strftime (s, maxsize, format, tp, 0);
+  return my_strftime (s, maxsize, format, tp, 1, 0);
 }
 #endif
 
 #if defined _LIBC && !defined COMPILE_WIDE
-weak_alias (__strftime_l, strftime_l)
+size_t
+__strftime_l_internal (char *s, size_t maxsize, const char *format,
+		       const struct tm *tp, __locale_t loc)
+{
+  return my_strftime (s, maxsize, format, tp, 1, loc);
+}
+strong_alias (__strftime_l_internal, __strftime_l_internal_alias)
+versioned_symbol (libc, __strftime_l_internal_alias,
+		  __strftime_l, GLIBC_2_26);
+libc_hidden_ver (__strftime_l_internal_alias, __strftime_l)
+versioned_symbol (libc, __strftime_l_internal, strftime_l, GLIBC_2_26);
+libc_hidden_ver (__strftime_l_internal, strftime_l)
+
+# if SHLIB_COMPAT (libc, GLIBC_2_3, GLIBC_2_26)
+size_t
+attribute_compat_text_section
+__strftime_l_compat (char *s, size_t maxsize, const char *format,
+		     const struct tm *tp, __locale_t loc)
+{
+  return my_strftime (s, maxsize, format, tp, 0, loc);
+}
+strong_alias (__strftime_l_compat, __strftime_l_compat_alias)
+compat_symbol (libc, __strftime_l_compat_alias, __strftime_l, GLIBC_2_3);
+compat_symbol (libc, __strftime_l_compat, strftime_l, GLIBC_2_3);
+# endif
+
 #endif
diff --git a/time/wcsftime.c b/time/wcsftime.c
index cbc4161..cf3371b 100644
--- a/time/wcsftime.c
+++ b/time/wcsftime.c
@@ -17,12 +17,26 @@ 
 
 #include <wchar.h>
 #include <locale/localeinfo.h>
+#include <shlib-compat.h>
 
 
 size_t
-wcsftime (wchar_t *s, size_t maxsize, const wchar_t *format,
-	  const struct tm *tp)
+__wcsftime (wchar_t *s, size_t maxsize, const wchar_t *format,
+	    const struct tm *tp)
 {
-  return __wcsftime_l (s, maxsize, format, tp, _NL_CURRENT_LOCALE);
+  return __wcsftime_l_common (s, maxsize, format, tp, 1, _NL_CURRENT_LOCALE);
 }
-libc_hidden_def (wcsftime)
+versioned_symbol (libc, __wcsftime, wcsftime, GLIBC_2_26);
+libc_hidden_ver (__wcsftime, wcsftime)
+
+
+#if SHLIB_COMPAT (libc, GLIBC_2_2, GLIBC_2_26)
+size_t
+attribute_compat_text_section
+__wcsftime_compat (wchar_t *s, size_t maxsize, const wchar_t *format,
+		   const struct tm *tp)
+{
+  return __wcsftime_l_common (s, maxsize, format, tp, 0, _NL_CURRENT_LOCALE);
+}
+compat_symbol (libc, __wcsftime_compat, wcsftime, GLIBC_2_2);
+#endif
diff --git a/time/wcsftime_l.c b/time/wcsftime_l.c
index 3210106..0a380bf 100644
--- a/time/wcsftime_l.c
+++ b/time/wcsftime_l.c
@@ -22,4 +22,28 @@ 
 #define COMPILE_WIDE	1
 #include "strftime_l.c"
 
-weak_alias (__wcsftime_l, wcsftime_l)
+size_t
+__wcsftime_l_internal (wchar_t *s, size_t maxsize, const wchar_t *format,
+		       const struct tm *tp, __locale_t loc)
+{
+  return my_strftime (s, maxsize, format, tp, 1, loc);
+}
+strong_alias (__wcsftime_l_internal, __wcsftime_l_internal_alias)
+versioned_symbol (libc, __wcsftime_l_internal_alias,
+		  __wcsftime_l, GLIBC_2_26);
+libc_hidden_ver (__wcsftime_l_internal_alias, __wcsftime_l)
+versioned_symbol (libc, __wcsftime_l_internal, wcsftime_l, GLIBC_2_26);
+libc_hidden_ver (__wcsftime_l_internal, wcsftime_l)
+
+#if SHLIB_COMPAT (libc, GLIBC_2_3, GLIBC_2_26)
+size_t
+attribute_compat_text_section
+__wcsftime_l_compat (wchar_t *s, size_t maxsize, const wchar_t *format,
+		     const struct tm *tp, __locale_t loc)
+{
+  return my_strftime (s, maxsize, format, tp, 0, loc);
+}
+strong_alias (__wcsftime_l_compat, __wcsftime_l_compat_alias)
+compat_symbol (libc, __wcsftime_l_compat_alias, __wcsftime_l, GLIBC_2_3);
+compat_symbol (libc, __wcsftime_l_compat, wcsftime_l, GLIBC_2_3);
+#endif