bug#54785: for floating point, printf should use double like in C instead of long double

[Paul Eggert]

Vincent Lefevre wrote in :


$ zsh -fc '/usr/bin/printf "%a\n" $((43./2**22))'
0xa.c00025cp-20

instead of

0xa.cp-20


To summarize, this test case is:

printf '%a\n' 1.0251998901367188e-05

and the problem is that converting 1.0251998901367188e-05 to long double 
prints the too-precise "0xa.c00025cp-20", whereas you want it to 
convert to double (which matches what most other programs do) and to 
print "0xa.cp-20" or equivalent.



(Note that ksh uses long double internally, but does not ensure the
round trip back to long double


Yes, ksh messes up here. However, it's more important for Coreutils 
printf to be compatible with the GNU shell, and Bash uses long double:


$ echo $BASH_VERSION
5.1.8(1)-release
$ /usr/bin/printf --version | head -n1
printf (GNU coreutils) 8.32
$ printf '%a\n' 1.0251998901367188e-05
0xa.c00025cp-20
$ /usr/bin/printf '%a\n' 1.0251998901367188e-05
0xa.c00025cp-20



I suggest to parse the argument as a "long double" only if the "L"
length modifier is provided, like in C.

Thanks, good idea.

I checked, and this also appears to be a POSIX conformance issue. POSIX 
 says that floating point operands "shall be evaluated as if by the 
strtod() function". This means double, not long double.


Whatever decision we make here, we should be consistent with Bash so 
I'll cc this email to bug-bash.


I propose that we change both coreutils and Bash to use 'double' rather 
than 'long double' here, unless the user specifies the L modifier (e.g., 
"printf '%La\n' ...". I've written up a patch (attached) to Bash 5.2 
alpha to do that. Assuming the Bash maintainer likes this proposal, I 
plan to implement something similar for Coreutils printf.diff '-x*~' -pru bash-5.2-alpha/builtins/printf.def bash-5.2-alpha-double/builtins/printf.def
--- bash-5.2-alpha/builtins/printf.def	2021-12-29 13:09:20.0 -0800
+++ bash-5.2-alpha-double/builtins/printf.def	2022-04-09 12:02:35.330476097 -0700
@@ -215,13 +215,14 @@ static uintmax_t getuintmax PARAMS((void
 
 #if defined (HAVE_LONG_DOUBLE) && HAVE_DECL_STRTOLD && !defined(STRTOLD_BROKEN)
 typedef long double floatmax_t;
-#  define FLOATMAX_CONV	"L"
+#  define USE_LONG_DOUBLE 1
 #  define strtofltmax	strtold
 #else
 typedef double floatmax_t;
-#  define FLOATMAX_CONV	""
+#  define USE_LONG_DOUBLE 0
 #  define strtofltmax	strtod
 #endif
+static double getdouble PARAMS((void));
 static floatmax_t getfloatmax PARAMS((void));
 
 static intmax_t asciicode PARAMS((void));
@@ -247,7 +248,7 @@ printf_builtin (list)
  WORD_LIST *list;
 {
   int ch, fieldwidth, precision;
-  int have_fieldwidth, have_precision;
+  int have_fieldwidth, have_precision, use_Lmod;
   char convch, thisch, nextch, *format, *modstart, *precstart, *fmt, *start;
 #if defined (HANDLE_MULTIBYTE)
   char mbch[25];		/* 25 > MB_LEN_MAX, plus can handle 4-byte UTF-8 and large Unicode characters*/
@@ -422,8 +423,12 @@ printf_builtin (list)
 
 	  /* skip possible format modifiers */
 	  modstart = fmt;
+	  use_Lmod = 0;
 	  while (*fmt && strchr (LENMODS, *fmt))
-	fmt++;
+	{
+	  use_Lmod |= USE_LONG_DOUBLE && *fmt == 'L';
+	  fmt++;
+	}
 	
 	  if (*fmt == 0)
 	{
@@ -694,11 +699,24 @@ printf_builtin (list)
 #endif
 	  {
 		char *f;
-		floatmax_t p;
 
-		p = getfloatmax ();
-		f = mklong (start, FLOATMAX_CONV, sizeof(FLOATMAX_CONV) - 1);
-		PF (f, p);
+		if (use_Lmod)
+		  {
+		floatmax_t p;
+
+		p = getfloatmax ();
+		f = mklong (start, "L", 1);
+		PF (f, p);
+		  }
+		else
+		  {
+		double p;
+
+		p = getdouble ();
+		f = mklong (start, "", 0);
+		PF (f, p);
+		  }
+
 		break;
 	  }
 
@@ -1248,35 +1266,40 @@ getuintmax ()
   return (ret);
 }
 
+#define getfloat(ret, convert) \
+  char *ep; \
+  if (garglist == 0) \
+return 0; \
+  if (garglist->word->word[0] == '\'' || garglist->word->word[0] == '"') \
+return asciicode (); \
+  errno = 0; \
+  (ret) = (convert) (garglist->word->word, &ep); \
+  if (*ep) \
+{ \
+  sh_invalidnum (garglist->word->word); \
+  /* Same thing about POSIX.2 conversion error requirements. */ \
+  if (0) \
+(ret) = 0; \
+  conversion_error = 1; \
+} \
+  else if (errno == ERANGE) \
+printf_erange (garglist->word->word); \
+  garglist = garglist->next
+
+static double
+getdouble ()
+{
+  double ret;
+  getfloat (ret, strtod);
+  return ret;
+}
+
 static floatmax_t
 getfloatmax ()
 {
   floatmax_t ret;
-  char *ep;
-
-  if (garglist == 0)
-return (0);
-
-  if (garglist->word->word[0] == '\'' || garglist->word->word[0] == '"')
-return asciicode ();
-
-  errno = 0;
-  ret = strtofltmax (garglist->word->word, &ep);
-
-  if (*ep)
-{
-  sh_invalidnum (garglist->word->word);
-#if 0
-  /* Same thing about POSIX.2 conversion error requirements. */
-  ret = 0;
-#endif
-  conversion_error = 1;
-}
-  else if (errno == ERANGE)
-printf_erange (garglist->word->word);
-
-  ga

bug#50889: Please improve the documentation for install --compare (-C)

[Pádraig Brady]

On 29/09/2021 12:09, Ian Cross via GNU coreutils Bug Reports wrote:

Hi,

A while back a request was made to improve the documentation of the
`install --compare` cmd:

https://lists.gnu.org/archive/html/bug-coreutils/2009-09/msg00029.html

It seems the wording in `coreutils.texi` has been updated since
(https://github.com/coreutils/coreutils/commit/c8ac385299950ba84eb8c33f7e32e4d85b18e3ff
) but the manpage and `--help` as of coreutils v8.32 still leave things
vague:


compare each pair of source and destination files, and in some cases,

do not modify the destination at all

Florain Schlichting at the time of the request suggested this wording
instead:


compare each pair of source and destination files, and if identical

(by content, ownership and mode), do not copy to preserve mtime

It looks like a patch was made but somehow never made it into the
codebase. Please could the manpage and help be updated with this
wording so users can understand more readily what compare will do?


Both the --help and texinfo was incomplete / confusing.
I've installed the attached to address this.

Marking this as done.

thanks,
Pádraig
From 6d246fb68b7d8fcd6e7b3da88bd786b305f40f32 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C3=A1draig=20Brady?= 
Date: Sat, 9 Apr 2022 12:42:37 +0100
Subject: [PATCH] doc: install --compare: clarify mode of operation

* doc/coreutils.texi (install invocation): For the --compare option,
clarify that the ownership or permissions of the source files don't
matter.  Also don't imply --owner or --group need to be specified
for --compare to be effective.
* src/install.c (usage): Add more detail on what's being compared.
---
 doc/coreutils.texi | 4 ++--
 src/install.c  | 5 +++--
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/doc/coreutils.texi b/doc/coreutils.texi
index c9243c683..8cfa698a1 100644
--- a/doc/coreutils.texi
+++ b/doc/coreutils.texi
@@ -9796,8 +9796,8 @@ The program accepts the following options.  Also see @ref{Common options}.
 @itemx --compare
 @opindex -C
 @opindex --compare
-Compare each pair of source and destination files, and if the destination has
-identical content and any specified owner, group, permissions, and possibly
+Compare content of source and destination files, and if there would be no
+change to the destination content, owner, group, permissions, and possibly
 SELinux context, then do not modify the destination at all.
 Note this option is best used in conjunction with @option{--user},
 @option{--group} and @option{--mode} options, lest @command{install}
diff --git a/src/install.c b/src/install.c
index 57a877f4a..079ce1f70 100644
--- a/src/install.c
+++ b/src/install.c
@@ -592,8 +592,9 @@ In the 4th form, create all components of the given DIRECTORY(ies).\n\
   --backup[=CONTROL]  make a backup of each existing destination file\n\
   -b  like --backup but does not accept an argument\n\
   -c  (ignored)\n\
-  -C, --compare   compare each pair of source and destination files, and\n\
-in some cases, do not modify the destination at all\n\
+  -C, --compare   compare content of source and destination files, and\n\
+if no change to content, ownership, and permissions,\n\
+do not modify the destination at all\n\
   -d, --directory treat all arguments as directory names; create all\n\
 components of the specified directories\n\
 "), stdout);
-- 
2.26.2