Release of version 4.14.2 of sharutils

[Bruce Korb]

GNU sharutils consists of two pairs of utilities: shar and unshar, and
uuencode and uudecode. "shar" makes so-called shell archives out of
many files, preparing them for transmission by electronic mail
services (converting binary data to ascii representations, breaking
the text into multiple shar scripts, etc.). "unshar" is the safe way
to extract and reassemble the original files. It will automatically
strip off the mail headers and other introductory text.

"uuencode" and "uudecode" are programs that convert binary files into
ascii text so that the original data can pass through the email system
without having intermediate hosts "fixing" the files en route.

This is a bug fix release for version 4.14 of sharutils

Version 4.14.2 - December 2014, by Bruce Korb
* translation updates
* use ftello, fseeko and fflush
* augment copyright marks in shar.c and shar-std.def
* Happy 2014 copyright dates, just in time for 2015 :)
* fix up man pages (use newer doc generation code)

sharutils home: http://www.gnu.org/software/sharutils/
primary ftp: ftp://ftp.gnu.org/gnu/sharutils/
.tar.gz: ftp://ftp.gnu.org/gnu/sharutils/sharutils-4.14.2.tar.gz
bug reports: bug-gnu-utils at the usual GNU domain
(be sure to mention "sharutils" in the subject...
it helps to spot the message.)
bug archive: http://lists.gnu.org/mailman/listinfo/bug-gnu-utils/
maintainer: Bruce Korb - bkorb at the usual GNU domain

[Benno Schulenberg]

Hello Bruce,

Post by Bruce Korb
.tar.gz: ftp://ftp.gnu.org/gnu/sharutils/sharutils-4.14.2.tar.gz

The tarball isn't there. Somehow it got stored in a subdir: su-4.14.2:
https://ftp.gnu.org/gnu/sharutils/su-4.14.2/

You may wish to correct that before I announce it to translators?

Regards,

Benno

[Bruce Korb]

Post by Benno Schulenberg
Hello Bruce,

Post by Bruce Korb
.tar.gz: ftp://ftp.gnu.org/gnu/sharutils/sharutils-4.14.2.tar.gz

https://ftp.gnu.org/gnu/sharutils/su-4.14.2/
You may wish to correct that before I announce it to translators?

Do you know how to correct it?
I staged it in a su-4.14.2 subdirectory and ran a script that did the same :(
I'll poke around and see how to fix it. Thank you.

[Benno Schulenberg]

Post by Bruce Korb

Post by Benno Schulenberg
https://ftp.gnu.org/gnu/sharutils/su-4.14.2/
You may wish to correct that before I announce it to translators?

Do you know how to correct it?

No. I've announced it anyway. When the URL changes, let me know.

Regards,

Benno

[Eric Blake]

Post by Bruce Korb

Post by Benno Schulenberg
Hello Bruce,

Post by Bruce Korb
.tar.gz: ftp://ftp.gnu.org/gnu/sharutils/sharutils-4.14.2.tar.gz

https://ftp.gnu.org/gnu/sharutils/su-4.14.2/
You may wish to correct that before I announce it to translators?

Do you know how to correct it?
I staged it in a su-4.14.2 subdirectory and ran a script that did the
same :(
I'll poke around and see how to fix it. Thank you.

https://www.gnu.org/prep/maintain/html_node/Automated-FTP-Uploads.html#Automated-FTP-Uploads
describes what you can do when uploading; it may be easiest to just
upload a duplicate copy under the correct name (for consistency with all
other releases), and leave the one-off name intact (now that distros are
starting to point to the one-off name).

[Eric Blake]

Post by Bruce Korb
GNU sharutils consists of two pairs of utilities: shar and unshar, and
uuencode and uudecode. "shar" makes so-called shell archives out of
many files, preparing them for transmission by electronic mail
services (converting binary data to ascii representations, breaking
the text into multiple shar scripts, etc.). "unshar" is the safe way
to extract and reassemble the original files. It will automatically
strip off the mail headers and other introductory text.
"uuencode" and "uudecode" are programs that convert binary files into
ascii text so that the original data can pass through the email system
without having intermediate hosts "fixing" the files en route.
This is a bug fix release for version 4.14 of sharutils
Version 4.14.2 - December 2014, by Bruce Korb
* translation updates
* use ftello, fseeko and fflush
* augment copyright marks in shar.c and shar-std.def
* Happy 2014 copyright dates, just in time for 2015 :)
* fix up man pages (use newer doc generation code)

You still have a bug that needs fixing:

shar.c: In function 'walktree':
shar.c:577:5: warning: implicit declaration of function 'basename'
[-Wimplicit-function-declaration]
restore_name = basename (local_name_copy);
^
shar.c:577:18: warning: assignment makes pointer from integer without a
cast [enabled by default]
restore_name = basename (local_name_copy);
^

On 32-bit cygwin, 'int' and 'void*' are the same width, so the correct
thing happens in spite of the bad coding, and the testsuite passes; but
on 64-bit cygwin, since 'int' is truncated, and the memory layout
intentionally sticks heap pointers outside the first 4G, you are
corrupting the value returned by basename by integer truncation before
re-expansion back to a pointer, and this causes a testsuite failure:

FAIL: shar-3
============

/usr/src/sharutils-4.14.2-1.x86_64/build/src/unshar: Found no shell
commands in ../shar-3-0002.shar
/tmp/shar-3-2868.d/base-2868 and shar-3.d differ
...

My guess is that shar-3-0002.shar is an empty file, because shar dumped
core when trying to dereference the truncated integer. By the way,
basename() is non-portable, and not thread-safe; it's better to use
gnulib's base_name() function.

You also ought to fix this warning:

unshar.c: In function 'load_file':
unshare.c:318:21: warning: initialization discards 'const' qualifier
from pointer target type [enabled by default]
char * pz_tmp = get_env_tmpdir();
^
although I don't think that one is fatal.

[Eric Blake]

Post by Eric Blake
shar.c:577:5: warning: implicit declaration of function 'basename'
[-Wimplicit-function-declaration]
restore_name = basename (local_name_copy);
^
shar.c:577:18: warning: assignment makes pointer from integer without a
cast [enabled by default]
restore_name = basename (local_name_copy);
^
On 32-bit cygwin, 'int' and 'void*' are the same width, so the correct
thing happens in spite of the bad coding, and the testsuite passes; but
on 64-bit cygwin, since 'int' is truncated, and the memory layout
intentionally sticks heap pointers outside the first 4G, you are
corrupting the value returned by basename by integer truncation before

Sure enough, this patch was sufficient to avoid the testsuite failure on
64-bit cygwin:

diff --git i/src/shar.c w/src/shar.c
index fd7cf41..47ad514 100644
--- i/src/shar.c
+++ w/src/shar.c
@@ -50,6 +50,7 @@ static const char cright_years_z[] =
# include <limits.h>
#endif
#include <time.h>
+#include <libgen.h>

#include "inttostr.h"
#include "liballoca.h"


That said,

Post by Eric Blake
basename() is non-portable, and not thread-safe; it's better to use
gnulib's base_name() function.

I still stand by this advice; gnulib intentionally does not guarantee
<libgen.h> (and it is missing on at least mingw), because basename() is
not the right function to be using if you care about systems with drive
letters.

[Bruce Korb]

Thank you. I think it is probably just easier to roll my own.
"basename" not being an especially difficult function to write.
Anyway, I'll look at it come the new year as I am leaving in a few minutes.
Thanks again

Post by Eric Blake

Post by Eric Blake
shar.c:577:5: warning: implicit declaration of function 'basename'
[-Wimplicit-function-declaration]
restore_name = basename (local_name_copy);
^
shar.c:577:18: warning: assignment makes pointer from integer without a
cast [enabled by default]
restore_name = basename (local_name_copy);
^
On 32-bit cygwin, 'int' and 'void*' are the same width, so the correct
thing happens in spite of the bad coding, and the testsuite passes; but
on 64-bit cygwin, since 'int' is truncated, and the memory layout
intentionally sticks heap pointers outside the first 4G, you are
corrupting the value returned by basename by integer truncation before

Sure enough, this patch was sufficient to avoid the testsuite failure on
diff --git i/src/shar.c w/src/shar.c
index fd7cf41..47ad514 100644
--- i/src/shar.c
+++ w/src/shar.c
@@ -50,6 +50,7 @@ static const char cright_years_z[] =
# include <limits.h>
#endif
#include <time.h>
+#include <libgen.h>
#include "inttostr.h"
#include "liballoca.h"
That said,

Post by Eric Blake
basename() is non-portable, and not thread-safe; it's better to use
gnulib's base_name() function.

I still stand by this advice; gnulib intentionally does not guarantee
<libgen.h> (and it is missing on at least mingw), because basename() is
not the right function to be using if you care about systems with drive
letters.

[Bruce Korb]

Post by Eric Blake
shar.c:577:5: warning: implicit declaration of function 'basename'
[-Wimplicit-function-declaration]
restore_name = basename (local_name_copy);
^

OK, I've incorporated use of base_name and fixed the "const" attribute.
Anything else before an official re-release?
(I'm incorporating new PO files, of course.)
The version without new PO files can be found here:
http://autogen.sourceforge.net/data/sharutils-4.14.3.tar.xz

[Paul Eggert]

Post by Bruce Korb
Anything else before an official re-release?

Please update the copyright dates to say '2015' rather than '2014'.

[Eric Blake]

Post by Paul Eggert

Post by Bruce Korb
Anything else before an official re-release?

Please update the copyright dates to say '2015' rather than '2014'.

Did you rebuild the tarball since Paul's request? I downloaded today,
and things like './src/shar --version' show a 2015 copyright date, as
does src/shar.c. So that part looked fine.

[Eric Blake]

Post by Bruce Korb
OK, I've incorporated use of base_name and fixed the "const" attribute.

No, you still missed a spot where you need to use base_name; and you
still have some other portability bugs. Read on...

Post by Bruce Korb
Anything else before an official re-release?

I built on 32-bit cygwin, and everything went fine (no build warnings
with the default warning level, all tests passed).

I then repeated on 64-bit cygwin, and did not see any compiler warnings
by default, but still failed one of the tests. Trying again with '-Wall
-Wextra' added to $CFLAGS, I saw a number of ctype usage bugs that you
ought to fix, although most users these days don't pick locales that
would trigger the buggy behavior:

CC libopts_a-libopts.o
In file included from libopts.c:32:0:
makeshell.c: In function 'emit_usage:
makeshell.c:399:13: warning: array subscript has type 'char'
[-Wchar-subscripts]
if ((*pzPN++ = (char)tolower(*pz++)) == NUL)
^
and several similar warnings (line 656, 659, 908 in makeshell.c, also
line 1583, 1584, 1595, 1569, 2082, 2091 of shar.c)

Here's an explanation of why this is indeed a (corner-case) bug - 'char'
on cygwin is a signed type, which means tolower((char)255) is
indistinguishable from tolower(-1) (due to the way sign-extension of
signed char is defined), but cygwin's definition of EOF is -1, so it
must return EOF. However, it is feasible to have a single-byte locale
where character 255 has different properties than EOF (that is, where
tolower((unsigned char)255) returns some other positive value).
Similarly, calling tolower((char)254) behaves as if it were tolower(-2)
which is undefined behavior according to POSIX (on cygwin, it happens to
have sane semantics of mimicking tolower((int)254), because cygwin
happens to map the lookups relative to the middle of a 384-byte table
where offsets [-128..-2] mirror [128-254] such that negative offsets
give the same results as their positive counterpart - only EOF has to
behave differently than 255) but that is not universally guaranteed on
other platforms).

Thus, when using tolower() and other <ctype.h> functions (which are all
locale-sensitive), you absolutely must convert the input argument
(*pz++) to unsigned char before calling the function. I suggest looking
at coreutils' src/system.h:to_uchar() as a nicer way to coerce ctype
arguments to the correct type than using plain casts.

Also, remember that tolower() doesn't always play nice in multibyte
locales; you may want to decide if it is right to make this code
locale-independent and multi-byte-safe by using gnulib's <c-ctype.h> and
c_tolower(), although it limits the set of case conversions that will be
performed to just ascii characters even in locales that have other case
mappings defined.

Meanwhile, here's the bug that caused the testsuite failure:

shar.c: In function 'split_shar_ed_file':
shar.c:1535:12: warning: implicit declaration of function 'basename'
[-Wimplicit-function-declaration]
basename (output_filename), part_number,
^
shar.c:1538:12: warning: format '%s' expects argument of type 'char *',
but argument 3 has type 'int' [-Wformat=]
restore, sharpid);
^

Here's another bug that should be fixed (a 64-bit ssize_t can cause the
fprintf to misbehave):

scribble.c: In function 'xscribble_get':
scribble.c:166:9: warning: format '%u' expects argument of type
'unsigned int', but argument 3 has type 'ssize_t' [-Wformat=]
fprintf(stderr, _("could not allocate %u bytes of scribble
space"), sz);
^

Finally, there were several harmless warnings, such as:

shar.c: In function 'format_report':
shar.c:345:25: warning: unused parameter 'type' [-Wunused-parameter]
format_report(quot_id_t type, char const * fmt, char const * what)
^

where you could use gcc attributes to shut it up (gnulib uses _GL_UNUSED
as a macro that expands to the right thing, if you want to try that), and:

unshar.c:62:41: warning: "/*" within comment [-Wcomment]
* Check for start of comment ("//" or "/*") and directives.
^

except that I don't know how you could reword that comment to still
convey the same information without causing grief to (non-compliant)
compilers that try to parse it as nested comments.

[Bruce Korb]

Post by Eric Blake

Post by Bruce Korb
OK, I've incorporated use of base_name and fixed the "const" attribute.

No, you still missed a spot where you need to use base_name; and you
still have some other portability bugs. Read on...

Well, I did where it was pointed out. I was lazy and went where pointed.

Post by Eric Blake
Also, remember that tolower() doesn't always play nice in multibyte
locales; you may want to decide if it is right to make this code
locale-independent and multi-byte-safe by using gnulib's <c-ctype.h> and
c_tolower(), although it limits the set of case conversions that will be
performed to just ascii characters even in locales that have other case
mappings defined.

At some point, it must also be remembered that the whole sharutils thing
is an old (make that, "ancient") hack for a world before MIME.
That feels like a bridge too far. :)

Post by Eric Blake
shar.c:345:25: warning: unused parameter 'type' [-Wunused-parameter]
format_report(quot_id_t type, char const * fmt, char const * what)

It looks like the quot_id_t stuff is unnecessary in that function and
can be removed instead of quieted. Likely a dinkleberry.

Post by Eric Blake
unshar.c:62:41: warning: "/*" within comment [-Wcomment]
* Check for start of comment ("//" or "/*") and directives.
^
except that I don't know how you could reword that comment to still
convey the same information without causing grief to (non-compliant)
compilers that try to parse it as nested comments.

Indeed. I think Doxycomments take priority here. I could if-def it
away, but that uglifies the code.