Google Groups no longer supports new Usenet posts or subscriptions. Historical content remains viewable.
Dismiss
Building sharutils 4.13.4 with MinGW
33 views
Skip to first unread message
Eli Zaretskii
Apr 7, 2013, 1:12:18 PM4/7/13
to bug-gn...@gnu.org
I've built this distribution with MinGW tools on MS-Windows. Quite
unexpectedly for such a veteran package, it turned out to be an uphill
battle.
In addition to MinGW-specific issues (which I will describe in a
separate message), I found a few issues that could show up on other
systems:
1. configure errors out:
configure: error: you must have sys_mman.h on your system
This is because libopts/m4/libopts.m4 does this:
for f in sys_types sys_mman sys_param sys_stat sys_wait \
string errno stdlib memory setjmp
do eval as_ac_var=\${ac_cv_header_${f}_h}
test "X${as_ac_var}" = Xyes || {
]AC_MSG_ERROR([you must have ${f}.h on your system])[
}
done
I don't understand why configure insists on having sys/mman.h and
sys/wait.h, because the code has appropriate fallbacks for when
they are not present. My solution was to remove sys_mman and
sys_wait from the above list.
2. There's a bug in text_mmap.c (in the part that falls back to
reading a file if mmap is unavailable): the txt_zero_fd member of
tmap_info_t structure was not initialized to -1, so stayed at zero
and was closed (twice) by close_mmap_files, which caused an invalid
argument debug message from the runtime library.
My solution was this:
--- sharutils-4.13.4.orig/libopts/text_mmap.c 2013-03-31 19:56:09.000000000 +0300
+++ sharutils-4.13.4/libopts/text_mmap.c 2013-04-04 16:30:08.429263900 +0300
@@ -233,7 +233,7 @@ close_mmap_files(tmap_info_t * mi)
close(mi->txt_fd);
mi->txt_fd = AO_INVALID_FD;
-#if ! defined(MAP_ANONYMOUS)
+#if defined(HAVE_MMAP) && ! defined(MAP_ANONYMOUS)
if (mi->txt_zero_fd == AO_INVALID_FD)
return;
3. Using the -R option seems to bypass encoding and only records the
options to the RC file -- is that intended? If so, it seems to be
undocumented (I initially thought it was a bug, and only by
stepping through the code found out that uudecode -R simply exits
after outputting the options to the RC file).
4. When passed 2 or more base64-encoded files, uudecode barfs on the
second one:
d:\usr\eli\utils\sharutils-4.13.4>src\uudecode bar2.b64 bar3.b64
uudecode fatal error:
bar3.b64: Invalid or missing 'begin' line
This is because of the following snippet in decode:
if (strncmp (buf, "begin", 5) == 0)
{
char * scan = buf+5;
if (*scan == '-')
{
static char const base64[] = "ase64";
static char const encoded[] = "encoded";
check_begin_option:
if (*++scan == 'b')
{
if (strncmp (scan+1, base64, sizeof (base64) - 1) != 0)
goto bad_beginning;
if (do_base64)
goto bad_beginning; <<<<<<<<<<<<<<<<<<<<<<<
do_base64 = true;
scan += sizeof (base64); /* chars + 'b' */
do_base64 is a static variable that doesn't get reset after
processing each file.
My solution was to reset do_base64 after each file finishes
processing:
--- sharutils-4.13.4.orig/src/uudecode.c 2013-03-29 17:28:40.000000000 +0300
+++ sharutils-4.13.4/src/uudecode.c 2013-04-07 09:38:10.818181900 +0300
@@ -497,6 +518,7 @@ multiple input files.\n"));
error (0, errno, "%s", f);
exit_status |= UUDECODE_EXIT_NO_INPUT;
}
+ do_base64 = false;
}
}
5. Question: can /dev/stdout or - appear on the encoded file's begin
line? If it can, then processing several files when one of them
has such a line will not redirect output back to the original
stream, because this snippet:
if ( (strcmp (outname, "/dev/stdout") != 0)
&& (strcmp (outname, "-") != 0) )
{
rval = reopen_output (outname, mode);
if (rval != UUDECODE_EXIT_SUCCESS)
goto fail_return;
}
doesn't redirect stdout in that case. So it looks like uudecode
will continue writing to the previously decoded file, instead of
resetting stdout to the original stream/descriptor.
6. shar fails to record the time and the submitter of the archive:
# Made on by <>.
The time was missing because print_header_stamp assumed %Z format
in strftime always takes at most 4 characters:
static char const ftime_fmt[] = "%Y-%m-%d %H:%M %Z";
/*
* All fields are two characters, except %Y is four.
*/
char buffer[sizeof (ftime_fmt) + 4]; <<<<<<<<<<<<<<<
time_t now;
struct tm * local_time;
time (&now);
local_time = localtime (&now);
strftime (buffer, sizeof (buffer) - 1, ftime_fmt, local_time);
which is false on Windows; the code was not testing the return
value of strftime, so it didn't see the falure.
My solution was to reallocate the buffer if strftime returns zero:
--- sharutils-4.13.4.orig/src/shar.c 2013-03-23 00:23:48.000000000 +0200
+++ sharutils-4.13.4/src/shar.c 2013-04-07 13:41:14.742531000 +0300
@@ -916,13 +921,20 @@ print_header_stamp (FILE * fp)
static char const ftime_fmt[] = "%Y-%m-%d %H:%M %Z";
/*
* All fields are two characters, except %Y is four.
+ * The length of %Z is implementation-defined.
*/
- char buffer[sizeof (ftime_fmt) + 4];
+ size_t bsize = sizeof (ftime_fmt) + 4;
+ char *buffer = alloca (bsize);
time_t now;
struct tm * local_time;
+ size_t len;
time (&now);
local_time = localtime (&now);
- strftime (buffer, sizeof (buffer) - 1, ftime_fmt, local_time);
+ while ((len = strftime (buffer, bsize - 1, ftime_fmt, local_time)) == 0)
+ {
+ bsize += 40;
+ buffer = alloca (bsize);
+ }
fprintf (fp, made_on_comment_z, buffer, OPT_ARG(SUBMITTER));
}
The problem with submitter was because set_submitter assigned a
local buffer to the submitter option argument, without using
xstrdup:
static void
set_submitter (void)
{
char buffer[256]; <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
char * uname = getuser (getuid ());
size_t len = strlen (uname);
if (uname == NULL)
fserr (SHAR_EXIT_FAILED, "getpwuid", "getuid()");
memcpy (buffer, uname, len);
buffer[len++] = '@';
gethostname (buffer + len, sizeof (buffer) - len);
SET_OPT_SUBMITTER(buffer); <<<<<<<<<<<<<<<<<<<<<<<<<
}
Solution: use xstrdup inside the SET_OPT_SUBMITTER call.
HTH
P.S. I'm not subscribed to this list, so please CC me on the
responses.
P.P.S. Thanks for maintaining sharutils!
unexpectedly for such a veteran package, it turned out to be an uphill
battle.
In addition to MinGW-specific issues (which I will describe in a
separate message), I found a few issues that could show up on other
systems:
1. configure errors out:
configure: error: you must have sys_mman.h on your system
This is because libopts/m4/libopts.m4 does this:
for f in sys_types sys_mman sys_param sys_stat sys_wait \
string errno stdlib memory setjmp
do eval as_ac_var=\${ac_cv_header_${f}_h}
test "X${as_ac_var}" = Xyes || {
]AC_MSG_ERROR([you must have ${f}.h on your system])[
}
done
I don't understand why configure insists on having sys/mman.h and
sys/wait.h, because the code has appropriate fallbacks for when
they are not present. My solution was to remove sys_mman and
sys_wait from the above list.
2. There's a bug in text_mmap.c (in the part that falls back to
reading a file if mmap is unavailable): the txt_zero_fd member of
tmap_info_t structure was not initialized to -1, so stayed at zero
and was closed (twice) by close_mmap_files, which caused an invalid
argument debug message from the runtime library.
My solution was this:
--- sharutils-4.13.4.orig/libopts/text_mmap.c 2013-03-31 19:56:09.000000000 +0300
+++ sharutils-4.13.4/libopts/text_mmap.c 2013-04-04 16:30:08.429263900 +0300
@@ -233,7 +233,7 @@ close_mmap_files(tmap_info_t * mi)
close(mi->txt_fd);
mi->txt_fd = AO_INVALID_FD;
-#if ! defined(MAP_ANONYMOUS)
+#if defined(HAVE_MMAP) && ! defined(MAP_ANONYMOUS)
if (mi->txt_zero_fd == AO_INVALID_FD)
return;
3. Using the -R option seems to bypass encoding and only records the
options to the RC file -- is that intended? If so, it seems to be
undocumented (I initially thought it was a bug, and only by
stepping through the code found out that uudecode -R simply exits
after outputting the options to the RC file).
4. When passed 2 or more base64-encoded files, uudecode barfs on the
second one:
d:\usr\eli\utils\sharutils-4.13.4>src\uudecode bar2.b64 bar3.b64
uudecode fatal error:
bar3.b64: Invalid or missing 'begin' line
This is because of the following snippet in decode:
if (strncmp (buf, "begin", 5) == 0)
{
char * scan = buf+5;
if (*scan == '-')
{
static char const base64[] = "ase64";
static char const encoded[] = "encoded";
check_begin_option:
if (*++scan == 'b')
{
if (strncmp (scan+1, base64, sizeof (base64) - 1) != 0)
goto bad_beginning;
if (do_base64)
goto bad_beginning; <<<<<<<<<<<<<<<<<<<<<<<
do_base64 = true;
scan += sizeof (base64); /* chars + 'b' */
do_base64 is a static variable that doesn't get reset after
processing each file.
My solution was to reset do_base64 after each file finishes
processing:
--- sharutils-4.13.4.orig/src/uudecode.c 2013-03-29 17:28:40.000000000 +0300
+++ sharutils-4.13.4/src/uudecode.c 2013-04-07 09:38:10.818181900 +0300
@@ -497,6 +518,7 @@ multiple input files.\n"));
error (0, errno, "%s", f);
exit_status |= UUDECODE_EXIT_NO_INPUT;
}
+ do_base64 = false;
}
}
5. Question: can /dev/stdout or - appear on the encoded file's begin
line? If it can, then processing several files when one of them
has such a line will not redirect output back to the original
stream, because this snippet:
if ( (strcmp (outname, "/dev/stdout") != 0)
&& (strcmp (outname, "-") != 0) )
{
rval = reopen_output (outname, mode);
if (rval != UUDECODE_EXIT_SUCCESS)
goto fail_return;
}
doesn't redirect stdout in that case. So it looks like uudecode
will continue writing to the previously decoded file, instead of
resetting stdout to the original stream/descriptor.
6. shar fails to record the time and the submitter of the archive:
# Made on by <>.
The time was missing because print_header_stamp assumed %Z format
in strftime always takes at most 4 characters:
static char const ftime_fmt[] = "%Y-%m-%d %H:%M %Z";
/*
* All fields are two characters, except %Y is four.
*/
char buffer[sizeof (ftime_fmt) + 4]; <<<<<<<<<<<<<<<
time_t now;
struct tm * local_time;
time (&now);
local_time = localtime (&now);
strftime (buffer, sizeof (buffer) - 1, ftime_fmt, local_time);
which is false on Windows; the code was not testing the return
value of strftime, so it didn't see the falure.
My solution was to reallocate the buffer if strftime returns zero:
--- sharutils-4.13.4.orig/src/shar.c 2013-03-23 00:23:48.000000000 +0200
+++ sharutils-4.13.4/src/shar.c 2013-04-07 13:41:14.742531000 +0300
@@ -916,13 +921,20 @@ print_header_stamp (FILE * fp)
static char const ftime_fmt[] = "%Y-%m-%d %H:%M %Z";
/*
* All fields are two characters, except %Y is four.
+ * The length of %Z is implementation-defined.
*/
- char buffer[sizeof (ftime_fmt) + 4];
+ size_t bsize = sizeof (ftime_fmt) + 4;
+ char *buffer = alloca (bsize);
time_t now;
struct tm * local_time;
+ size_t len;
time (&now);
local_time = localtime (&now);
- strftime (buffer, sizeof (buffer) - 1, ftime_fmt, local_time);
+ while ((len = strftime (buffer, bsize - 1, ftime_fmt, local_time)) == 0)
+ {
+ bsize += 40;
+ buffer = alloca (bsize);
+ }
fprintf (fp, made_on_comment_z, buffer, OPT_ARG(SUBMITTER));
}
The problem with submitter was because set_submitter assigned a
local buffer to the submitter option argument, without using
xstrdup:
static void
set_submitter (void)
{
char buffer[256]; <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
char * uname = getuser (getuid ());
size_t len = strlen (uname);
if (uname == NULL)
fserr (SHAR_EXIT_FAILED, "getpwuid", "getuid()");
memcpy (buffer, uname, len);
buffer[len++] = '@';
gethostname (buffer + len, sizeof (buffer) - len);
SET_OPT_SUBMITTER(buffer); <<<<<<<<<<<<<<<<<<<<<<<<<
}
Solution: use xstrdup inside the SET_OPT_SUBMITTER call.
HTH
P.S. I'm not subscribed to this list, so please CC me on the
responses.
P.P.S. Thanks for maintaining sharutils!
Eli Zaretskii
Apr 7, 2013, 1:35:50 PM4/7/13
to bug-gn...@gnu.org, bug-g...@gnu.org
Here are the MinGW-specific problems with building and testing this
release (copy to gnulib list, because many issues are related to
gnulib modules).
1. Compilation fails in lib/:
gcc -DHAVE_CONFIG_H -I. -I.. -I../intl -Id:/usr/include -g3 -O2 -Wno-format-contains-nul -MT idcache.o -MD -MP -MF .deps/idcache.Tpo -c -o idcache.o idcache.c
In file included from idcache.c:21:0:
idcache.h:6:23: error: unknown type name 'uid_t'
idcache.h:7:24: error: unknown type name 'gid_t'
idcache.h:8:1: error: unknown type name 'uid_t'
idcache.h:9:1: error: unknown type name 'gid_t'
idcache.c:25:17: fatal error: pwd.h: No such file or directory
compilation terminated.
make[4]: *** [idcache.o] Error 1
This is because idcache.h includes sys/types.h, but gnulib's
sys/types.h does not define uid_t and gid_t, and also there are no
pwd.h and grp.h in lib/.
My solution was to add private pwd.h and grp.h to lib/, and modify
idcache.h to include them. But I guess a better solution would be
to import more from gnulib, or perhaps enhance the corresponding
gnulib modules.
2. Compilation warning in lib/:
gcc -DHAVE_CONFIG_H -I. -I.. -I../intl -Id:/usr/include -g3 -O2 -Wno-format-contains-nul -MT idcache.o -MD -MP -MF .deps/idcache.Tpo -c -o idcache.o idcache.c
idcache.c: In function 'getuser':
idcache.c:85:30: warning: initialization makes pointer from integer without a cast [enabled by default]
idcache.c: In function 'getgroup':
idcache.c:167:29: warning: initialization makes pointer from integer without a cast [enabled by default]
This is because functions getpwuid and getgrgid are unavailable and
there are no replacements for them in gnulib.
To solve this, I added the missing functions to idcache.c:
--- sharutils-4.13.4.orig/lib/idcache.c 2013-03-31 21:03:14.000000000 +0300
+++ sharutils-4.13.4/lib/idcache.c 2013-04-03 12:51:30.807618000 +0300
@@ -62,6 +62,70 @@ static struct userid *group_alist;
/* Each entry on list is a group name for which the first lookup failed. */
static struct userid *nogroup_alist;
+#ifdef __MINGW32__
+#define WIN32_LEAN_AND_MEAN
+#include <windows.h>
+#include <lmcons.h>
+uid_t
+getuid (void)
+{
+ return 42;
+}
+
+struct passwd *
+getpwuid (uid_t uid)
+{
+ static char uname[UNLEN+1];
+ static char homedir[FILENAME_MAX];
+ DWORD unsz = sizeof (uname);
+ static struct passwd user_rec;
+
+ if (uname[0] == '\0')
+ GetUserName (uname, &unsz);
+ if (homedir[0] == '\0')
+ {
+ char *home = getenv ("HOME");
+
+ if (home)
+ strcpy (homedir, home);
+ else
+ {
+ home = getenv ("HOMEDRIVE");
+ if (home)
+ strcpy (homedir, home);
+ home = getenv ("HOMEPATH");
+ if (home)
+ strcat (homedir, home);
+ }
+ }
+ user_rec.pw_uid = uid;
+ user_rec.pw_name = uname;
+ user_rec.pw_dir = homedir;
+
+ return &user_rec;
+}
+
+struct passwd *
+getpwnam (const char *user)
+{
+ return getpwuid (1004);
+}
+
+struct group *
+getgrgid (gid_t gid)
+{
+ static struct group grp_rec = { 42, "Users" };
+
+ return &grp_rec;
+}
+
+struct group *
+getgrnam (const char *group)
+{
+ return getgrgid (42);
+}
+#endif
+
/* Translate UID to a login name, with cache, or NULL if unresolved. */
char *
3. Compilation warnings in src/shar.c:
gcc -DLOCALEDIR=\"d:/usr/share/locale\" -DHAVE_CONFIG_H -I. -I.. -I../libopts -I. -I.. -I../lib -I../intl -Id:/usr/include -Wno-format-contains-nul -g3 -O2 -Wno-format-contains-nul -MT shar.o -MD -MP -MF .deps/shar.Tpo -c -o shar.o shar.c
shar.c: In function 'walktree':
shar.c:595:18: warning: assignment makes pointer from integer without a cast [enabled by default]
shar.c: In function 'set_submitter':
shar.c:1943:18: warning: initialization makes pointer from integer without a cast [enabled by default]
make[2]: *** [shar.o] Error 1
The problem here is with basename and dirname, whose declarations
are absent. My solution was to add "#include <libgen.h>" to
lib/system.h (libgen.h in MinGW declares basename and dirname).
However, I wonder if gnulib has a better solution.
4. Link errors for shar.exe:
gcc -Wno-format-contains-nul -g3 -O2 -Wno-format-contains-nul -o shar.exe shar.o encode.o shar-opts.o -lws2_32 -lws2_32 ../libopts/libopts.a ../lib/libgnu.a ../intl/libintl.a d:/usr/lib/libiconv.dll.a -Ld:/usr/lib -lpthread ../lib/libgnu.a -lregex
shar.o: In function `shar':
d:\usr\eli\utils\sharutils-4.13.4\src/shar.c:1760: undefined reference to `wait'
shar.o: In function `open_shar_input':
d:\usr\eli\utils\sharutils-4.13.4\src/shar.c:1308: undefined reference to `pipe'
d:\usr\eli\utils\sharutils-4.13.4\src/shar.c:1312: undefined reference to `fork'
shar.o: In function `set_submitter':
d:\usr\eli\utils\sharutils-4.13.4\src/shar.c:1943: undefined reference to `getuid'
../lib/libgnu.a(idcache.o): In function `getuidbyname':
d:\usr\eli\utils\sharutils-4.13.4\lib/idcache.c:147: undefined reference to `getpwnam'
../lib/libgnu.a(idcache.o): In function `getgidbyname':
d:\usr\eli\utils\sharutils-4.13.4\lib/idcache.c:229: undefined reference to `getgrnam'
../lib/libgnu.a(gethostname.o): In function `rpl_gethostname':
d:\usr\eli\utils\sharutils-4.13.4\lib/gethostname.c:97: undefined reference to `gethostname@8'
../lib/libgnu.a(gethostname.o): In function `set_winsock_errno':
d:\usr\eli\utils\sharutils-4.13.4\lib/w32sock.h:37: undefined reference to `WSAGetLastError@0'
../lib/libgnu.a(sockets.o): In function `close_fd_maybe_socket':
d:\usr\eli\utils\sharutils-4.13.4\lib/sockets.c:51: undefined reference to `WSAEnumNetworkEvents@12'
d:\usr\eli\utils\sharutils-4.13.4\lib/sockets.c:59: undefined reference to `closesocket@4'
../lib/libgnu.a(sockets.o): In function `set_winsock_errno':
d:\usr\eli\utils\sharutils-4.13.4\lib/w32sock.h:37: undefined reference to `WSAGetLastError@0'
../lib/libgnu.a(sockets.o): In function `ioctl_fd_maybe_socket':
d:\usr\eli\utils\sharutils-4.13.4\lib/sockets.c:88: undefined reference to `WSAEnumNetworkEvents@12'
d:\usr\eli\utils\sharutils-4.13.4\lib/sockets.c:92: undefined reference to `ioctlsocket@12'
../lib/libgnu.a(sockets.o): In function `set_winsock_errno':
d:\usr\eli\utils\sharutils-4.13.4\lib/w32sock.h:37: undefined reference to `WSAGetLastError@0'
../lib/libgnu.a(sockets.o): In function `gl_sockets_startup':
d:\usr\eli\utils\sharutils-4.13.4\lib/sockets.c:120: undefined reference to `WSAStartup@8'
../lib/libgnu.a(sockets.o): In function `gl_sockets_cleanup':
d:\usr\eli\utils\sharutils-4.13.4\lib/sockets.c:148: undefined reference to `WSACleanup@0'
collect2.exe: error: ld returned 1 exit status
make[2]: *** [shar.exe] Error 1
make[2]: Leaving directory `/d/usr/eli/utils/sharutils-4.13.4/src'
make[1]: *** [all-recursive] Error 1
make[1]: Leaving directory `/d/usr/eli/utils/sharutils-4.13.4'
make: *** [all] Error 2
The winsock-related errors are because of incorrect order of
libraries on the link command line:
-lws2_32 -lws2_32 ../libopts/libopts.a ../lib/libgnu.a ../intl/libintl.a d:/usr/lib/libiconv.dll.a -Ld:/usr/lib -lpthread ../lib/libgnu.a -lregex
Solution: reorder in src/Makefile.in:
LDADD = $(top_builddir)/libopts/libopts.a $(top_builddir)/lib/libgnu.a \
$(LIBINTL) $(top_builddir)/lib/libgnu.a $(GETHOSTNAME_LIB) $(LIBSOCKET)
For the other errors: I wrote emulations for fork (always fails),
wait (always fails) and pipe (calls _pipe), and also add getuid,
getpwnam and getgrnam to idcache.c. However, I think gnulib has
replacements for at least some of these functions.
5. Link errors for unshar.exe:
gcc -Wno-format-contains-nul -g3 -O2 -Wno-format-contains-nul -o unshar.exe unshar.o encode.o unshar-opts.o ../libopts/libopts.a ../lib/libgnu.a ../intl/libintl.a d:/usr/lib/libiconv.dll.a -Ld:/usr/lib -lpthread ../lib/libgnu.a -lws2_32 -lws2_32 -lregex
unshar.o: In function `load_file':
d:\usr\eli\utils\sharutils-4.13.4\src/unshar.c:310: undefined reference to `mkstemp'
collect2.exe: error: ld returned 1 exit status
make[2]: *** [unshar.exe] Error 1
Solution: add mkstemp to unshar.c. Again, I think gnulib has a
module for that, so importing it will solve this.
6. uutest-1 test fails because the begin line doesn't match. This is
because the mode bits on Windows are not real, beyond the user-read
and user-write bits. So it would be nice if on Windows Diff would
be invoked like this:
DIFF="diff -I^begin"
7. shar-4 fails because it uses cmp to compare results with expected
ones, which fails due to line endings. Suggest to use Diff
instead.
8. The configure script botches the popen-binary test: the test
program does not include stdio.h, so the compilation fails for
reasons that have no relation to the issue being tested.
9. Using -! or --more-help triggers an error message:
d:\usr\eli\utils\sharutils-4.13.4>uuencode -!
uuencode: illegal option -- !
uuencode (GNU sharutils) - encode a file into email friendly text
Usage: uuencode [ -<flag> | --<name> ]... [<in-file>] <output-name>
Try 'uuencode --help' for more information.
d:\usr\eli\utils\sharutils-4.13.4>uuencode --more-help
uuencode: illegal option -- more-help
uuencode (GNU sharutils) - encode a file into email friendly text
Usage: uuencode [ -<flag> | --<name> ]... [<in-file>] <output-name>
Try 'uuencode --help' for more information.
This is confusing because --help does show these options as available.
The reason is that uuencode-options.c does this:
#if HAVE_WORKING_FORK
#define MORE_HELP_DESC (uuencode_opt_strs+1045)
#define MORE_HELP_name (uuencode_opt_strs+1090)
#define MORE_HELP_FLAGS (OPTST_IMM | OPTST_NO_INIT)
#else
#define MORE_HELP_DESC NULL <<<<<<<<<<<<<<<<<<<<<<<<<
#define MORE_HELP_name NULL <<<<<<<<<<<<<<<<<<<<<<<<<
#define MORE_HELP_FLAGS (OPTST_OMITTED | OPTST_NO_INIT)
#endif
which is unnecessary, since the relevant libopts code already knows
how to handle the case of !HAVE_WORKING_FORK.
=> Fixed by enabling the !HAVE_WORKING_FORK block above.
Same in shar-opts.c and uudecode-opts.c.
10.shar fails while processing files:
d:\usr\eli\utils\sharutils-4.13.4>src\shar src\encode.c > foo.shar
shar: Saving src/encode.c (text)
The system cannot find the path specified.
This is because emit_char_ct_validation invokes 'wc' with bad
redirection:
set LC_ALL=C & wc -c < 'src/encode.c'
The file from which stdin is redirected uses forward slashes and is
quoted with '..'.
Solution: use a slightly different command for MinGW to run 'wc'
locally:
set LC_ALL=C & wc -c "src/encode.c"
--- sharutils-4.13.4.orig/src/shar.c 2013-03-23 00:23:48.000000000 +0200
+++ sharutils-4.13.4/src/shar.c 2013-04-07 13:41:14.742531000 +0300
@@ -1065,14 +1097,27 @@ emit_char_ct_validation (
/* Shell command able to count characters from its standard input.
We have to take care for the locale setting because wc in multi-byte
character environments gets different results. */
+#ifdef __MINGW32__
+ /* Windows needs a slightly different format to run wc locally:
+ redirection will fail because the file uses Unix forward
+ slashes. */
+ static char const local_cmd[] = "set LC_ALL=C & wc -c";
+#endif
static char const cct_cmd[] = "LC_ALL=C wc -c <";
FILE *pfp;
char wc[16 /* log MAX_INT + a few extra */];
- char *command = alloca (sizeof(cct_cmd) + strlen (quoted_name) + 2);
+#ifdef __MINGW32__
+ /* On Windows, disregard Unix-style shell quoting and quote by hand
+ using "..". */
+ char *command = alloca (sizeof(local_cmd) + strlen (local_name) + 2);
+ sprintf (command, "%s \"%s\"", local_cmd, local_name);
+#else
+ char *command = alloca (sizeof(cct_cmd) + strlen (quoted_name) + 2);
sprintf (command, "%s %s", cct_cmd, quoted_name);
- pfp = popen (command, "r");
+#endif
+ pfp = popen (command, freadonly_mode);
if (pfp == NULL)
die (SHAR_EXIT_FAILED, _("Could not popen command"), command);
11.shar.c uses /dev/null in freopen, but does not use the gnulib
freopen module, which makes /dev/null work on MS-Windows.
There are a few other, less important issues, like with using binary
I/O in some places. let me know if you want me to send diffs for
those.
HTH
release (copy to gnulib list, because many issues are related to
gnulib modules).
1. Compilation fails in lib/:
gcc -DHAVE_CONFIG_H -I. -I.. -I../intl -Id:/usr/include -g3 -O2 -Wno-format-contains-nul -MT idcache.o -MD -MP -MF .deps/idcache.Tpo -c -o idcache.o idcache.c
In file included from idcache.c:21:0:
idcache.h:6:23: error: unknown type name 'uid_t'
idcache.h:7:24: error: unknown type name 'gid_t'
idcache.h:8:1: error: unknown type name 'uid_t'
idcache.h:9:1: error: unknown type name 'gid_t'
idcache.c:25:17: fatal error: pwd.h: No such file or directory
compilation terminated.
make[4]: *** [idcache.o] Error 1
This is because idcache.h includes sys/types.h, but gnulib's
sys/types.h does not define uid_t and gid_t, and also there are no
pwd.h and grp.h in lib/.
My solution was to add private pwd.h and grp.h to lib/, and modify
idcache.h to include them. But I guess a better solution would be
to import more from gnulib, or perhaps enhance the corresponding
gnulib modules.
2. Compilation warning in lib/:
gcc -DHAVE_CONFIG_H -I. -I.. -I../intl -Id:/usr/include -g3 -O2 -Wno-format-contains-nul -MT idcache.o -MD -MP -MF .deps/idcache.Tpo -c -o idcache.o idcache.c
idcache.c: In function 'getuser':
idcache.c:85:30: warning: initialization makes pointer from integer without a cast [enabled by default]
idcache.c: In function 'getgroup':
idcache.c:167:29: warning: initialization makes pointer from integer without a cast [enabled by default]
This is because functions getpwuid and getgrgid are unavailable and
there are no replacements for them in gnulib.
To solve this, I added the missing functions to idcache.c:
--- sharutils-4.13.4.orig/lib/idcache.c 2013-03-31 21:03:14.000000000 +0300
+++ sharutils-4.13.4/lib/idcache.c 2013-04-03 12:51:30.807618000 +0300
@@ -62,6 +62,70 @@ static struct userid *group_alist;
/* Each entry on list is a group name for which the first lookup failed. */
static struct userid *nogroup_alist;
+#ifdef __MINGW32__
+#define WIN32_LEAN_AND_MEAN
+#include <windows.h>
+#include <lmcons.h>
+uid_t
+getuid (void)
+{
+ return 42;
+}
+
+struct passwd *
+getpwuid (uid_t uid)
+{
+ static char uname[UNLEN+1];
+ static char homedir[FILENAME_MAX];
+ DWORD unsz = sizeof (uname);
+ static struct passwd user_rec;
+
+ if (uname[0] == '\0')
+ GetUserName (uname, &unsz);
+ if (homedir[0] == '\0')
+ {
+ char *home = getenv ("HOME");
+
+ if (home)
+ strcpy (homedir, home);
+ else
+ {
+ home = getenv ("HOMEDRIVE");
+ if (home)
+ strcpy (homedir, home);
+ home = getenv ("HOMEPATH");
+ if (home)
+ strcat (homedir, home);
+ }
+ }
+ user_rec.pw_uid = uid;
+ user_rec.pw_name = uname;
+ user_rec.pw_dir = homedir;
+
+ return &user_rec;
+}
+
+struct passwd *
+getpwnam (const char *user)
+{
+ return getpwuid (1004);
+}
+
+struct group *
+getgrgid (gid_t gid)
+{
+ static struct group grp_rec = { 42, "Users" };
+
+ return &grp_rec;
+}
+
+struct group *
+getgrnam (const char *group)
+{
+ return getgrgid (42);
+}
+#endif
+
/* Translate UID to a login name, with cache, or NULL if unresolved. */
char *
3. Compilation warnings in src/shar.c:
gcc -DLOCALEDIR=\"d:/usr/share/locale\" -DHAVE_CONFIG_H -I. -I.. -I../libopts -I. -I.. -I../lib -I../intl -Id:/usr/include -Wno-format-contains-nul -g3 -O2 -Wno-format-contains-nul -MT shar.o -MD -MP -MF .deps/shar.Tpo -c -o shar.o shar.c
shar.c: In function 'walktree':
shar.c:595:18: warning: assignment makes pointer from integer without a cast [enabled by default]
shar.c: In function 'set_submitter':
shar.c:1943:18: warning: initialization makes pointer from integer without a cast [enabled by default]
make[2]: *** [shar.o] Error 1
The problem here is with basename and dirname, whose declarations
are absent. My solution was to add "#include <libgen.h>" to
lib/system.h (libgen.h in MinGW declares basename and dirname).
However, I wonder if gnulib has a better solution.
4. Link errors for shar.exe:
gcc -Wno-format-contains-nul -g3 -O2 -Wno-format-contains-nul -o shar.exe shar.o encode.o shar-opts.o -lws2_32 -lws2_32 ../libopts/libopts.a ../lib/libgnu.a ../intl/libintl.a d:/usr/lib/libiconv.dll.a -Ld:/usr/lib -lpthread ../lib/libgnu.a -lregex
shar.o: In function `shar':
d:\usr\eli\utils\sharutils-4.13.4\src/shar.c:1760: undefined reference to `wait'
shar.o: In function `open_shar_input':
d:\usr\eli\utils\sharutils-4.13.4\src/shar.c:1308: undefined reference to `pipe'
d:\usr\eli\utils\sharutils-4.13.4\src/shar.c:1312: undefined reference to `fork'
shar.o: In function `set_submitter':
d:\usr\eli\utils\sharutils-4.13.4\src/shar.c:1943: undefined reference to `getuid'
../lib/libgnu.a(idcache.o): In function `getuidbyname':
d:\usr\eli\utils\sharutils-4.13.4\lib/idcache.c:147: undefined reference to `getpwnam'
../lib/libgnu.a(idcache.o): In function `getgidbyname':
d:\usr\eli\utils\sharutils-4.13.4\lib/idcache.c:229: undefined reference to `getgrnam'
../lib/libgnu.a(gethostname.o): In function `rpl_gethostname':
d:\usr\eli\utils\sharutils-4.13.4\lib/gethostname.c:97: undefined reference to `gethostname@8'
../lib/libgnu.a(gethostname.o): In function `set_winsock_errno':
d:\usr\eli\utils\sharutils-4.13.4\lib/w32sock.h:37: undefined reference to `WSAGetLastError@0'
../lib/libgnu.a(sockets.o): In function `close_fd_maybe_socket':
d:\usr\eli\utils\sharutils-4.13.4\lib/sockets.c:51: undefined reference to `WSAEnumNetworkEvents@12'
d:\usr\eli\utils\sharutils-4.13.4\lib/sockets.c:59: undefined reference to `closesocket@4'
../lib/libgnu.a(sockets.o): In function `set_winsock_errno':
d:\usr\eli\utils\sharutils-4.13.4\lib/w32sock.h:37: undefined reference to `WSAGetLastError@0'
../lib/libgnu.a(sockets.o): In function `ioctl_fd_maybe_socket':
d:\usr\eli\utils\sharutils-4.13.4\lib/sockets.c:88: undefined reference to `WSAEnumNetworkEvents@12'
d:\usr\eli\utils\sharutils-4.13.4\lib/sockets.c:92: undefined reference to `ioctlsocket@12'
../lib/libgnu.a(sockets.o): In function `set_winsock_errno':
d:\usr\eli\utils\sharutils-4.13.4\lib/w32sock.h:37: undefined reference to `WSAGetLastError@0'
../lib/libgnu.a(sockets.o): In function `gl_sockets_startup':
d:\usr\eli\utils\sharutils-4.13.4\lib/sockets.c:120: undefined reference to `WSAStartup@8'
../lib/libgnu.a(sockets.o): In function `gl_sockets_cleanup':
d:\usr\eli\utils\sharutils-4.13.4\lib/sockets.c:148: undefined reference to `WSACleanup@0'
collect2.exe: error: ld returned 1 exit status
make[2]: *** [shar.exe] Error 1
make[2]: Leaving directory `/d/usr/eli/utils/sharutils-4.13.4/src'
make[1]: *** [all-recursive] Error 1
make[1]: Leaving directory `/d/usr/eli/utils/sharutils-4.13.4'
make: *** [all] Error 2
The winsock-related errors are because of incorrect order of
libraries on the link command line:
-lws2_32 -lws2_32 ../libopts/libopts.a ../lib/libgnu.a ../intl/libintl.a d:/usr/lib/libiconv.dll.a -Ld:/usr/lib -lpthread ../lib/libgnu.a -lregex
Solution: reorder in src/Makefile.in:
LDADD = $(top_builddir)/libopts/libopts.a $(top_builddir)/lib/libgnu.a \
$(LIBINTL) $(top_builddir)/lib/libgnu.a $(GETHOSTNAME_LIB) $(LIBSOCKET)
For the other errors: I wrote emulations for fork (always fails),
wait (always fails) and pipe (calls _pipe), and also add getuid,
getpwnam and getgrnam to idcache.c. However, I think gnulib has
replacements for at least some of these functions.
5. Link errors for unshar.exe:
gcc -Wno-format-contains-nul -g3 -O2 -Wno-format-contains-nul -o unshar.exe unshar.o encode.o unshar-opts.o ../libopts/libopts.a ../lib/libgnu.a ../intl/libintl.a d:/usr/lib/libiconv.dll.a -Ld:/usr/lib -lpthread ../lib/libgnu.a -lws2_32 -lws2_32 -lregex
unshar.o: In function `load_file':
d:\usr\eli\utils\sharutils-4.13.4\src/unshar.c:310: undefined reference to `mkstemp'
collect2.exe: error: ld returned 1 exit status
make[2]: *** [unshar.exe] Error 1
Solution: add mkstemp to unshar.c. Again, I think gnulib has a
module for that, so importing it will solve this.
6. uutest-1 test fails because the begin line doesn't match. This is
because the mode bits on Windows are not real, beyond the user-read
and user-write bits. So it would be nice if on Windows Diff would
be invoked like this:
DIFF="diff -I^begin"
7. shar-4 fails because it uses cmp to compare results with expected
ones, which fails due to line endings. Suggest to use Diff
instead.
8. The configure script botches the popen-binary test: the test
program does not include stdio.h, so the compilation fails for
reasons that have no relation to the issue being tested.
9. Using -! or --more-help triggers an error message:
d:\usr\eli\utils\sharutils-4.13.4>uuencode -!
uuencode: illegal option -- !
uuencode (GNU sharutils) - encode a file into email friendly text
Usage: uuencode [ -<flag> | --<name> ]... [<in-file>] <output-name>
Try 'uuencode --help' for more information.
d:\usr\eli\utils\sharutils-4.13.4>uuencode --more-help
uuencode: illegal option -- more-help
uuencode (GNU sharutils) - encode a file into email friendly text
Usage: uuencode [ -<flag> | --<name> ]... [<in-file>] <output-name>
Try 'uuencode --help' for more information.
This is confusing because --help does show these options as available.
The reason is that uuencode-options.c does this:
#if HAVE_WORKING_FORK
#define MORE_HELP_DESC (uuencode_opt_strs+1045)
#define MORE_HELP_name (uuencode_opt_strs+1090)
#define MORE_HELP_FLAGS (OPTST_IMM | OPTST_NO_INIT)
#else
#define MORE_HELP_DESC NULL <<<<<<<<<<<<<<<<<<<<<<<<<
#define MORE_HELP_name NULL <<<<<<<<<<<<<<<<<<<<<<<<<
#define MORE_HELP_FLAGS (OPTST_OMITTED | OPTST_NO_INIT)
#endif
which is unnecessary, since the relevant libopts code already knows
how to handle the case of !HAVE_WORKING_FORK.
=> Fixed by enabling the !HAVE_WORKING_FORK block above.
Same in shar-opts.c and uudecode-opts.c.
10.shar fails while processing files:
d:\usr\eli\utils\sharutils-4.13.4>src\shar src\encode.c > foo.shar
shar: Saving src/encode.c (text)
The system cannot find the path specified.
This is because emit_char_ct_validation invokes 'wc' with bad
redirection:
set LC_ALL=C & wc -c < 'src/encode.c'
The file from which stdin is redirected uses forward slashes and is
quoted with '..'.
Solution: use a slightly different command for MinGW to run 'wc'
locally:
set LC_ALL=C & wc -c "src/encode.c"
--- sharutils-4.13.4.orig/src/shar.c 2013-03-23 00:23:48.000000000 +0200
+++ sharutils-4.13.4/src/shar.c 2013-04-07 13:41:14.742531000 +0300
/* Shell command able to count characters from its standard input.
We have to take care for the locale setting because wc in multi-byte
character environments gets different results. */
+#ifdef __MINGW32__
+ /* Windows needs a slightly different format to run wc locally:
+ redirection will fail because the file uses Unix forward
+ slashes. */
+ static char const local_cmd[] = "set LC_ALL=C & wc -c";
+#endif
static char const cct_cmd[] = "LC_ALL=C wc -c <";
FILE *pfp;
char wc[16 /* log MAX_INT + a few extra */];
- char *command = alloca (sizeof(cct_cmd) + strlen (quoted_name) + 2);
+#ifdef __MINGW32__
+ /* On Windows, disregard Unix-style shell quoting and quote by hand
+ using "..". */
+ char *command = alloca (sizeof(local_cmd) + strlen (local_name) + 2);
+ sprintf (command, "%s \"%s\"", local_cmd, local_name);
+#else
+ char *command = alloca (sizeof(cct_cmd) + strlen (quoted_name) + 2);
sprintf (command, "%s %s", cct_cmd, quoted_name);
- pfp = popen (command, "r");
+#endif
+ pfp = popen (command, freadonly_mode);
if (pfp == NULL)
die (SHAR_EXIT_FAILED, _("Could not popen command"), command);
11.shar.c uses /dev/null in freopen, but does not use the gnulib
freopen module, which makes /dev/null work on MS-Windows.
There are a few other, less important issues, like with using binary
I/O in some places. let me know if you want me to send diffs for
those.
HTH
Andreas Schwab
Apr 7, 2013, 1:48:21 PM4/7/13
to Eli Zaretskii, bug-gn...@gnu.org
Eli Zaretskii <el...@gnu.org> writes:
> 6. shar fails to record the time and the submitter of the archive:
>
> # Made on by <>.
>
> The time was missing because print_header_stamp assumed %Z format
> in strftime always takes at most 4 characters:
>
> static char const ftime_fmt[] = "%Y-%m-%d %H:%M %Z";
> /*
> * All fields are two characters, except %Y is four.
> */
> char buffer[sizeof (ftime_fmt) + 4]; <<<<<<<<<<<<<<<
> time_t now;
> struct tm * local_time;
> time (&now);
> local_time = localtime (&now);
> strftime (buffer, sizeof (buffer) - 1, ftime_fmt, local_time);
>
> which is false on Windows;
It is even wrong for Unix, where timezone names can have as much as
> 6. shar fails to record the time and the submitter of the archive:
>
> # Made on by <>.
>
> The time was missing because print_header_stamp assumed %Z format
> in strftime always takes at most 4 characters:
>
> static char const ftime_fmt[] = "%Y-%m-%d %H:%M %Z";
> /*
> * All fields are two characters, except %Y is four.
> */
> char buffer[sizeof (ftime_fmt) + 4]; <<<<<<<<<<<<<<<
> time_t now;
> struct tm * local_time;
> time (&now);
> local_time = localtime (&now);
> strftime (buffer, sizeof (buffer) - 1, ftime_fmt, local_time);
>
> which is false on Windows;
TZNAME_MAX (typically 6) characters, and a couple of current zones in
the Olsen database have more than 4 characters, let alone a lot of
historical ones.
Andreas.
--
Andreas Schwab, sch...@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."
Eli Zaretskii
Apr 7, 2013, 1:55:56 PM4/7/13
to Andreas Schwab, bug-gn...@gnu.org
> From: Andreas Schwab <sch...@linux-m68k.org>
> Cc: bug-gn...@gnu.org
> Date: Sun, 07 Apr 2013 19:48:21 +0200
FWIW.) Windows time zones have names much longer than 6; I see a few
that are longer than 30.
> Cc: bug-gn...@gnu.org
> Date: Sun, 07 Apr 2013 19:48:21 +0200
>
> > static char const ftime_fmt[] = "%Y-%m-%d %H:%M %Z";
> > /*
> > * All fields are two characters, except %Y is four.
> > */
> > char buffer[sizeof (ftime_fmt) + 4]; <<<<<<<<<<<<<<<
> > time_t now;
> > struct tm * local_time;
> > time (&now);
> > local_time = localtime (&now);
> > strftime (buffer, sizeof (buffer) - 1, ftime_fmt, local_time);
> >
> > which is false on Windows;
>
> It is even wrong for Unix, where timezone names can have as much as
> TZNAME_MAX (typically 6) characters, and a couple of current zones in
> the Olsen database have more than 4 characters, let alone a lot of
> historical ones.
Thanks, I didn't know about TZNAME_MAX. (It doesn't exist on Windows,
> > static char const ftime_fmt[] = "%Y-%m-%d %H:%M %Z";
> > /*
> > * All fields are two characters, except %Y is four.
> > */
> > char buffer[sizeof (ftime_fmt) + 4]; <<<<<<<<<<<<<<<
> > time_t now;
> > struct tm * local_time;
> > time (&now);
> > local_time = localtime (&now);
> > strftime (buffer, sizeof (buffer) - 1, ftime_fmt, local_time);
> >
> > which is false on Windows;
>
> It is even wrong for Unix, where timezone names can have as much as
> TZNAME_MAX (typically 6) characters, and a couple of current zones in
> the Olsen database have more than 4 characters, let alone a lot of
> historical ones.
FWIW.) Windows time zones have names much longer than 6; I see a few
that are longer than 30.
Bruce Korb
Apr 7, 2013, 4:46:00 PM4/7/13
to Eli Zaretskii, bug-g...@gnu.org, Andreas Schwab, bug-gn...@gnu.org
On 04/07/13 10:35, Eli Zaretskii wrote:
> Here are the MinGW-specific problems with building and testing this
> release (copy to gnulib list, because many issues are related to
> gnulib modules).
>
> 1. Compilation fails in lib/:
>
> Here are the MinGW-specific problems with building and testing this
> release (copy to gnulib list, because many issues are related to
> gnulib modules).
>
> 1. Compilation fails in lib/:
>
> This is because idcache.h includes sys/types.h, but gnulib's
> sys/types.h does not define uid_t and gid_t, and also there are no
> pwd.h and grp.h in lib/.
>
> My solution was to add private pwd.h and grp.h to lib/, and modify
> idcache.h to include them. But I guess a better solution would be
> to import more from gnulib, or perhaps enhance the corresponding
> gnulib modules.
Indeed. "enhance the corresponding gnulib modules" because gnulib is
> sys/types.h does not define uid_t and gid_t, and also there are no
> pwd.h and grp.h in lib/.
>
> My solution was to add private pwd.h and grp.h to lib/, and modify
> idcache.h to include them. But I guess a better solution would be
> to import more from gnulib, or perhaps enhance the corresponding
> gnulib modules.
supposed to recursively include everything needed.
> 2. Compilation warning in lib/:
> This is because functions getpwuid and getgrgid are unavailable and
> there are no replacements for them in gnulib.
>
> To solve this, I added the missing functions to idcache.c:
That would require gnulib cooperation, too.
> there are no replacements for them in gnulib.
>
> To solve this, I added the missing functions to idcache.c:
Otherwise, I have to glue it into sources I control.
> 3. Compilation warnings in src/shar.c:
>
> gcc -DLOCALEDIR=\"d:/usr/share/locale\" -DHAVE_CONFIG_H -I. -I.. -I../libopts -I. -I.. -I../lib -I../intl -Id:/usr/include -Wno-format-contains-nul -g3 -O2 -Wno-format-contains-nul -MT shar.o -MD -MP -MF .deps/shar.Tpo -c -o shar.o shar.c
> shar.c: In function 'walktree':
> shar.c:595:18: warning: assignment makes pointer from integer without a cast [enabled by default]
> shar.c: In function 'set_submitter':
> shar.c:1943:18: warning: initialization makes pointer from integer without a cast [enabled by default]
> make[2]: *** [shar.o] Error 1
>
> The problem here is with basename and dirname, whose declarations
> are absent. My solution was to add "#include <libgen.h>" to
> lib/system.h (libgen.h in MinGW declares basename and dirname).
> However, I wonder if gnulib has a better solution.
> 4. Link errors for shar.exe:
Pasted into my Makefile.am.
> For the other errors: I wrote emulations for fork (always fails),
> wait (always fails) and pipe (calls _pipe), and also add getuid,
> getpwnam and getgrnam to idcache.c. However, I think gnulib has
> replacements for at least some of these functions.
> $ list=$(find * -type f)
> $ fgrep getuid $list
> $ fgrep getpwnam $list
> $ fgrep getgrnam $list
> $ echo "$list"|egrep "getuid|getpwnam|getgrnam"
> $
> 5. Link errors for unshar.exe:
> Solution: add mkstemp to unshar.c. Again, I think gnulib has a
> module for that, so importing it will solve this.
Done. (Added to gnulib modules)
> module for that, so importing it will solve this.
> 6. uutest-1 test fails because the begin line doesn't match. This is
> because the mode bits on Windows are not real, beyond the user-read
> and user-write bits. So it would be nice if on Windows Diff would
> be invoked like this:
>
> DIFF="diff -I^begin"
>
> 7. shar-4 fails because it uses cmp to compare results with expected
> ones, which fails due to line endings. Suggest to use Diff
> instead.
so that I know that I have to:
sed 's/^begin [0-7][0-7][0-7] /begin 664/'
the result file.
> 8. The configure script botches the popen-binary test: the test
> program does not include stdio.h, so the compilation fails for
> reasons that have no relation to the issue being tested.
> 9. Using -! or --more-help triggers an error message:
>
> d:\usr\eli\utils\sharutils-4.13.4>uuencode -!
> uuencode: illegal option -- !
> uuencode (GNU sharutils) - encode a file into email friendly text
> Usage: uuencode [ -<flag> | --<name> ]... [<in-file>] <output-name>
> Try 'uuencode --help' for more information.
> This is confusing because --help does show these options as available.
>
> The reason is that uuencode-options.c does this:
>
> #if HAVE_WORKING_FORK
> #define MORE_HELP_DESC (uuencode_opt_strs+1045)
> #define MORE_HELP_name (uuencode_opt_strs+1090)
> #define MORE_HELP_FLAGS (OPTST_IMM | OPTST_NO_INIT)
> #else
> #define MORE_HELP_DESC NULL <<<<<<<<<<<<<<<<<<<<<<<<<
> #define MORE_HELP_name NULL <<<<<<<<<<<<<<<<<<<<<<<<<
> #define MORE_HELP_FLAGS (OPTST_OMITTED | OPTST_NO_INIT)
> #endif
>
> which is unnecessary, since the relevant libopts code already knows
> how to handle the case of !HAVE_WORKING_FORK.
>
> => Fixed by enabling the !HAVE_WORKING_FORK block above.
>
> Same in shar-opts.c and uudecode-opts.c.
Lovely. This is a dilemma. In order to facilitate i18n, I cannot compute help
>
> The reason is that uuencode-options.c does this:
>
> #if HAVE_WORKING_FORK
> #define MORE_HELP_DESC (uuencode_opt_strs+1045)
> #define MORE_HELP_name (uuencode_opt_strs+1090)
> #define MORE_HELP_FLAGS (OPTST_IMM | OPTST_NO_INIT)
> #else
> #define MORE_HELP_DESC NULL <<<<<<<<<<<<<<<<<<<<<<<<<
> #define MORE_HELP_name NULL <<<<<<<<<<<<<<<<<<<<<<<<<
> #define MORE_HELP_FLAGS (OPTST_OMITTED | OPTST_NO_INIT)
> #endif
>
> which is unnecessary, since the relevant libopts code already knows
> how to handle the case of !HAVE_WORKING_FORK.
>
> => Fixed by enabling the !HAVE_WORKING_FORK block above.
>
> Same in shar-opts.c and uudecode-opts.c.
text on the fly. If I don't compute it on the fly, then it has to be the same
text for all platforms. I guess I should set it up to handle it -- only handle
it as if it were plain help. Rats.
All four *-opts.c have the issue. I've modified the source template:
> #ifdef HAVE_WORKING_FORK
> #define MORE_HELP_DESC ([=<junk>=])
> #define MORE_HELP_name ([=<more junk>=])
> #define MORE_HELP_FLAGS (OPTST_IMM | OPTST_NO_INIT)
> #else
> #define MORE_HELP_DESC HELP_DESC
> #else
> #define MORE_HELP_name HELP_name
> #define MORE_HELP_FLAGS (OPTST_OMITTED | OPTST_NO_INIT)
> #endif
I am also certain that is not everything. Basically, on platforms without fork(),
> #endif
you'll just get --help with --more-help.
> 10.shar fails while processing files:
>
> d:\usr\eli\utils\sharutils-4.13.4>src\shar src\encode.c > foo.shar
> shar: Saving src/encode.c (text)
> The system cannot find the path specified.
>
> This is because emit_char_ct_validation invokes 'wc' with bad
> redirection:
>
> set LC_ALL=C & wc -c < 'src/encode.c'
>
> The file from which stdin is redirected uses forward slashes and is
> quoted with '..'.
>
> Solution: use a slightly different command for MinGW to run 'wc'
> locally:
>
> set LC_ALL=C & wc -c "src/encode.c"
The ``quoted_name'' argument is the string "'src/encode.c'".
It is the same argument as is passed to finish_sharing_file.
Anyway, if in the end MinGW requires double quotes for path names
that might have directory characters, it is starting to look like
it might be better to:
local_name=`localize 'src/encode.c'`
LC_ALL=C wc -c < $local_name
with the "localize" function most commonly being "echo".
What is being emitted in this function is shell code that gets executed
on the receiving machine. In other words, shell code that can be interpreted
by shell-on-windows and shell-on-real-posix. Selecting the form of the
output based on __MINGW32__ at shar compile time is incorrect.
So the patch needed would be some boilerplate functions that behave differently
for Mingw and the rest of the world and it gets selected at unshar time.
This "wc -c" code gets replaced with that function call.
> 11.shar.c uses /dev/null in freopen, but does not use the gnulib
> freopen module, which makes /dev/null work on MS-Windows.
"Portable" POSIX is hard enough without some system that
masquerades as almost, kind-of-like POSIXy.
> There are a few other, less important issues, like with using binary
> I/O in some places. let me know if you want me to send diffs for
> those.
>
> HTH
> I've built this distribution with MinGW tools on MS-Windows. Quite
> unexpectedly for such a veteran package, it turned out to be an uphill
> battle.
change mandated by our Austin Group friends), plus the docs were all
out of date and things had gotten crufty. So I bit off too much.
> 1. configure errors out:
>
> configure: error: you must have sys_mman.h on your system
> I don't understand why configure insists on having sys/mman.h and
> sys/wait.h, because the code has appropriate fallbacks for when
> they are not present.
I think the correct fix is to do the gnulib futzing and then
> sys/wait.h, because the code has appropriate fallbacks for when
> they are not present.
set ac_cv_header_sys_wait_h and ac_cv_header_sys_mman_h to "yes".
The libopts stuff should follow that. (I don't think stuffing
gnulib stuff into libopts would work so well.)
> 2. There's a bug in text_mmap.c (in the part that falls back to
> reading a file if mmap is unavailable): the txt_zero_fd member of
> tmap_info_t structure was not initialized to -1, so stayed at zero
> and was closed (twice) by close_mmap_files, which caused an invalid
> argument debug message from the runtime library.
> 3. Using the -R option seems to bypass encoding and only records the
> options to the RC file -- is that intended? If so, it seems to be
> undocumented (I initially thought it was a bug, and only by
> stepping through the code found out that uudecode -R simply exits
> after outputting the options to the RC file).
This will show up in ntp and tls documentation, too:
The command will exit after updating the config file.
It basically turns the program into a config file editor.
> 4. When passed 2 or more base64-encoded files, uudecode barfs on the
> second one:
> --- sharutils-4.13.4.orig/src/uudecode.c 2013-03-29 17:28:40.000000000 +0300
> +++ sharutils-4.13.4/src/uudecode.c 2013-04-07 09:38:10.818181900 +0300
> @@ -497,6 +518,7 @@ multiple input files.\n"));
> error (0, errno, "%s", f);
> exit_status |= UUDECODE_EXIT_NO_INPUT;
> }
> + do_base64 = false;
> }
> }
Thanks. Fixed.
> +++ sharutils-4.13.4/src/uudecode.c 2013-04-07 09:38:10.818181900 +0300
> @@ -497,6 +518,7 @@ multiple input files.\n"));
> error (0, errno, "%s", f);
> exit_status |= UUDECODE_EXIT_NO_INPUT;
> }
> + do_base64 = false;
> }
> }
>
> 5. Question: can /dev/stdout or - appear on the encoded file's begin
> line? If it can, then
> doesn't redirect stdout in that case. So it looks like uudecode
> will continue writing to the previously decoded file, instead of
> resetting stdout to the original stream/descriptor.
That is an inherited feature. :) I likely wordsmith-ed the following,
> will continue writing to the previously decoded file, instead of
> resetting stdout to the original stream/descriptor.
but it is an old caveat:
> BUGS
> If more than one file is given to uudecode and the -o option is given
> or more than one name in the encoded files are the same the result is
> probably not what is expected.
> 6. shar fails to record the time and the submitter of the archive:
>
> # Made on by <>.
>
> The time was missing because print_header_stamp assumed %Z format
> in strftime always takes at most 4 characters:
static char const ftime_fmt[] = "%Y-%m-%d %H:%M %Z";
/*
* All fields are two characters, except %Y is four and
/*
* %Z may be up to 30 (?!?!)
*/
char buffer[sizeof (ftime_fmt) + 64];
> My solution was to reallocate the buffer if strftime returns zero:
size_t l =
strftime (buffer, sizeof (buffer) - 1, ftime_fmt, local_time);
if (l > 0)
fprintf (fp, made_on_comment_z, buffer, OPT_ARG(SUBMITTER));
}
> The problem with submitter was because set_submitter assigned a
> local buffer to the submitter option argument, without using
> xstrdup:
}
> The problem with submitter was because set_submitter assigned a
> local buffer to the submitter option argument, without using
> xstrdup:
> Solution: use xstrdup inside the SET_OPT_SUBMITTER call.
Hmm. I misremembered my own code. Rehacked:
> static void
> set_submitter (void)
> {
> char * buffer;
> char * uname = getuser (getuid ());
> size_t len = strlen (uname);
> if (uname == NULL)
> fserr (SHAR_EXIT_FAILED, "getpwuid", "getuid()");
> buffer = xmalloc (len + 2 + HOST_NAME_MAX);
> size_t len = strlen (uname);
> if (uname == NULL)
> fserr (SHAR_EXIT_FAILED, "getpwuid", "getuid()");
> memcpy (buffer, uname, len);
> buffer[len++] = '@';
> gethostname (buffer + len, HOST_NAME_MAX);
> buffer[len++] = '@';
> SET_OPT_SUBMITTER(buffer);
> }
> P.P.S. Thanks for maintaining sharutils!
It was supposed to be easy, and I went and made it harder.
> P.P.S. Thanks for maintaining sharutils!
Thank you.
http://autogen.sourceforge.net/data/
has both a "pre2" sharutils and the autogen for creating it.
(You do not need the latter, as the "libopts" gets rolled into sharutils.)
The issues that really need gnulib fixes are still in sharutils.
I'm also still trying to figure out how to select which
ac_cv_header_xxx_h
values should be set to "yes".
Thank you again. Regards, Bruce
Eli Zaretskii
Apr 8, 2013, 11:35:45 AM4/8/13
to Bruce Korb, bug-g...@gnu.org, bug-gn...@gnu.org
> Date: Sun, 07 Apr 2013 13:46:00 -0700
> From: Bruce Korb <bk...@gnu.org>
> CC: bug-gn...@gnu.org, bug-g...@gnu.org,
> Andreas Schwab <sch...@linux-m68k.org>
> > 6. uutest-1 test fails because the begin line doesn't match. This is
> > because the mode bits on Windows are not real, beyond the user-read
> > and user-write bits. So it would be nice if on Windows Diff would
> > be invoked like this:
> >
> > DIFF="diff -I^begin"
> >
> > 7. shar-4 fails because it uses cmp to compare results with expected
> > ones, which fails due to line endings. Suggest to use Diff
> > instead.
>
> Could you suggest a patch for these two, too? :) Maybe a script test
> so that I know that I have to:
> sed 's/^begin [0-7][0-7][0-7] /begin 664/'
> the result file.
The easiest way to test for this is to say "chmod a+w foo" and then
see that "ls -l foo" still shows it as "-rw-r--r--" (or anyway
something other than world-writable).
> > 9. Using -! or --more-help triggers an error message:
> >
> > d:\usr\eli\utils\sharutils-4.13.4>uuencode -!
> > uuencode: illegal option -- !
> > uuencode (GNU sharutils) - encode a file into email friendly text
> > Usage: uuencode [ -<flag> | --<name> ]... [<in-file>] <output-name>
> > Try 'uuencode --help' for more information.
>
> > This is confusing because --help does show these options as available.
> >
> > The reason is that uuencode-options.c does this:
> >
> > #if HAVE_WORKING_FORK
> > #define MORE_HELP_DESC (uuencode_opt_strs+1045)
> > #define MORE_HELP_name (uuencode_opt_strs+1090)
> > #define MORE_HELP_FLAGS (OPTST_IMM | OPTST_NO_INIT)
> > #else
> > #define MORE_HELP_DESC NULL <<<<<<<<<<<<<<<<<<<<<<<<<
> > #define MORE_HELP_name NULL <<<<<<<<<<<<<<<<<<<<<<<<<
> > #define MORE_HELP_FLAGS (OPTST_OMITTED | OPTST_NO_INIT)
> > #endif
> >
> > which is unnecessary, since the relevant libopts code already knows
> > how to handle the case of !HAVE_WORKING_FORK.
> >
> > => Fixed by enabling the !HAVE_WORKING_FORK block above.
> >
> > Same in shar-opts.c and uudecode-opts.c.
>
> Lovely. This is a dilemma. In order to facilitate i18n, I cannot compute help
> text on the fly. If I don't compute it on the fly, then it has to be the same
> text for all platforms.
My suggestion was unrelated to i18n, I think. All I was saying was
that if the help text cannot be piped through a pager, it should be
dumped to stdout instead. That is better than an error message, IMO.
> Basically, on platforms without fork(), you'll just get --help with
> --more-help.
That's exactly what I think should be done here, no more.
> > 10.shar fails while processing files:
> >
> > d:\usr\eli\utils\sharutils-4.13.4>src\shar src\encode.c > foo.shar
> > shar: Saving src/encode.c (text)
> > The system cannot find the path specified.
> >
> > This is because emit_char_ct_validation invokes 'wc' with bad
> > redirection:
> >
> > set LC_ALL=C & wc -c < 'src/encode.c'
> >
> > The file from which stdin is redirected uses forward slashes and is
> > quoted with '..'.
> >
> > Solution: use a slightly different command for MinGW to run 'wc'
> > locally:
> >
> > set LC_ALL=C & wc -c "src/encode.c"
>
> Hmmm. That routine is *called* with quoting it doesn't handle well.
> The ``quoted_name'' argument is the string "'src/encode.c'".
> It is the same argument as is passed to finish_sharing_file.
Yes, but the quoting caters to a Posix shell, not to what Windows
expects.
> Anyway, if in the end MinGW requires double quotes for path names
> that might have directory characters, it is starting to look like
> it might be better to:
> local_name=`localize 'src/encode.c'`
> LC_ALL=C wc -c < $local_name
> with the "localize" function most commonly being "echo".
This has nothing to do with localization. The Windows shell simply
does not support '..' kind of quoting.
I think there's a misunderstanding here. I'm aware that the wc
command gets written into the shar. My suggested patch didn't touch
that part, for that very reason, and as result the shar file still
includes the correct Posix shell command. Here's an example produced
by my build on Windows:
test `LC_ALL=C wc -c < 'gnu/sharutils-4.13.4/src/shar.c'` -ne 60556 && \
${echo} "restoration warning: size of 'gnu/sharutils-4.13.4/src/shar.c' is not 60556"
However, the function in question, emit_char_ct_validation, uses the
command constructed from cct_cmd[] also for running wc locally, via
popen, at shar creation time -- that's how it produces the
verification value (60556 in the example above). My changes touched
only the local invocation, not the verification command that gets
recorded in the shar and is supposed to run in a Unixy shell.
This problem is exacerbated by the fact that all the file names need
to have their backslash directory separators converted to Unix-style
forward slashes before they get to shar-creation code. (This is in a
part of my changes I didn't show.) This is because file names
recorded in the shar _must_ be Posix compatible. So even if the
quoting problem would somehow be solved (e.g., by adding a special
style to the quotearg module in gnulib), redirection will still not
work, because the Windows shell which is run by popen does not
understand forward slashes in file names.
So I think a Windows-specific local command for running wc at shar
creation time is the best compromise. Since the quoted strings are
file names, and Windows doesn't allow quote characters in file names,
simply enclosing each name in quotes is enough.
> > 11.shar.c uses /dev/null in freopen, but does not use the gnulib
> > freopen module, which makes /dev/null work on MS-Windows.
>
> Isn't there a POSIX layer that can be added to windows?
Not with MinGW. MinGW produces native Windows executable that depend
on the Windows runtime libraries.
In any case, gnulib has the freopen module which solves the /dev/null
problem. It just needs to be added to the package.
> > 1. configure errors out:
> >
> > configure: error: you must have sys_mman.h on your system
>
> > I don't understand why configure insists on having sys/mman.h and
> > sys/wait.h, because the code has appropriate fallbacks for when
> > they are not present.
>
> I think the correct fix is to do the gnulib futzing and then
> set ac_cv_header_sys_wait_h and ac_cv_header_sys_mman_h to "yes".
> The libopts stuff should follow that. (I don't think stuffing
> gnulib stuff into libopts would work so well.)
Sorry, I don't understand. The libopts stuff already has fallback
code for when sys/mman.h and mmap are missing -- see text_mmap.c,
which can cope with HAVE_MMAP being undefined. The only problem is
that libopts/m4/libopts.m4 explicitly insists on some headers being
present:
for f in sys_types sys_mman sys_param sys_stat sys_wait \
string errno stdlib memory setjmp
do eval as_ac_var=\${ac_cv_header_${f}_h}
test "X${as_ac_var}" = Xyes || {
]AC_MSG_ERROR([you must have ${f}.h on your system])[
} ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
done
Simply removing sys_mman and sys_wait from this list solves the
problem.
> http://autogen.sourceforge.net/data/
>
> has both a "pre2" sharutils and the autogen for creating it.
I'm not done yet ;-) See my other message.
> From: Bruce Korb <bk...@gnu.org>
> CC: bug-gn...@gnu.org, bug-g...@gnu.org,
> Andreas Schwab <sch...@linux-m68k.org>
>
> > For the other errors: I wrote emulations for fork (always fails),
> > wait (always fails) and pipe (calls _pipe), and also add getuid,
> > getpwnam and getgrnam to idcache.c. However, I think gnulib has
> > replacements for at least some of these functions.
>
> Not that I can find.
> > $ list=$(find * -type f)
> > $ fgrep getuid $list
> > $ fgrep getpwnam $list
> > $ fgrep getgrnam $list
> > $ echo "$list"|egrep "getuid|getpwnam|getgrnam"
> > $
I meant fork, wait (waitpid in gnulib), and pipe.
> > For the other errors: I wrote emulations for fork (always fails),
> > wait (always fails) and pipe (calls _pipe), and also add getuid,
> > getpwnam and getgrnam to idcache.c. However, I think gnulib has
> > replacements for at least some of these functions.
>
> Not that I can find.
> > $ list=$(find * -type f)
> > $ fgrep getuid $list
> > $ fgrep getpwnam $list
> > $ fgrep getgrnam $list
> > $ echo "$list"|egrep "getuid|getpwnam|getgrnam"
> > $
> > 6. uutest-1 test fails because the begin line doesn't match. This is
> > because the mode bits on Windows are not real, beyond the user-read
> > and user-write bits. So it would be nice if on Windows Diff would
> > be invoked like this:
> >
> > DIFF="diff -I^begin"
> >
> > 7. shar-4 fails because it uses cmp to compare results with expected
> > ones, which fails due to line endings. Suggest to use Diff
> > instead.
>
> Could you suggest a patch for these two, too? :) Maybe a script test
> so that I know that I have to:
> sed 's/^begin [0-7][0-7][0-7] /begin 664/'
> the result file.
see that "ls -l foo" still shows it as "-rw-r--r--" (or anyway
something other than world-writable).
> > 9. Using -! or --more-help triggers an error message:
> >
> > d:\usr\eli\utils\sharutils-4.13.4>uuencode -!
> > uuencode: illegal option -- !
> > uuencode (GNU sharutils) - encode a file into email friendly text
> > Usage: uuencode [ -<flag> | --<name> ]... [<in-file>] <output-name>
> > Try 'uuencode --help' for more information.
>
> > This is confusing because --help does show these options as available.
> >
> > The reason is that uuencode-options.c does this:
> >
> > #if HAVE_WORKING_FORK
> > #define MORE_HELP_DESC (uuencode_opt_strs+1045)
> > #define MORE_HELP_name (uuencode_opt_strs+1090)
> > #define MORE_HELP_FLAGS (OPTST_IMM | OPTST_NO_INIT)
> > #else
> > #define MORE_HELP_DESC NULL <<<<<<<<<<<<<<<<<<<<<<<<<
> > #define MORE_HELP_name NULL <<<<<<<<<<<<<<<<<<<<<<<<<
> > #define MORE_HELP_FLAGS (OPTST_OMITTED | OPTST_NO_INIT)
> > #endif
> >
> > which is unnecessary, since the relevant libopts code already knows
> > how to handle the case of !HAVE_WORKING_FORK.
> >
> > => Fixed by enabling the !HAVE_WORKING_FORK block above.
> >
> > Same in shar-opts.c and uudecode-opts.c.
>
> Lovely. This is a dilemma. In order to facilitate i18n, I cannot compute help
> text on the fly. If I don't compute it on the fly, then it has to be the same
> text for all platforms.
that if the help text cannot be piped through a pager, it should be
dumped to stdout instead. That is better than an error message, IMO.
> Basically, on platforms without fork(), you'll just get --help with
> --more-help.
> > 10.shar fails while processing files:
> >
> > d:\usr\eli\utils\sharutils-4.13.4>src\shar src\encode.c > foo.shar
> > shar: Saving src/encode.c (text)
> > The system cannot find the path specified.
> >
> > This is because emit_char_ct_validation invokes 'wc' with bad
> > redirection:
> >
> > set LC_ALL=C & wc -c < 'src/encode.c'
> >
> > The file from which stdin is redirected uses forward slashes and is
> > quoted with '..'.
> >
> > Solution: use a slightly different command for MinGW to run 'wc'
> > locally:
> >
> > set LC_ALL=C & wc -c "src/encode.c"
>
> Hmmm. That routine is *called* with quoting it doesn't handle well.
> The ``quoted_name'' argument is the string "'src/encode.c'".
> It is the same argument as is passed to finish_sharing_file.
expects.
> Anyway, if in the end MinGW requires double quotes for path names
> that might have directory characters, it is starting to look like
> it might be better to:
> local_name=`localize 'src/encode.c'`
> LC_ALL=C wc -c < $local_name
> with the "localize" function most commonly being "echo".
does not support '..' kind of quoting.
command gets written into the shar. My suggested patch didn't touch
that part, for that very reason, and as result the shar file still
includes the correct Posix shell command. Here's an example produced
by my build on Windows:
test `LC_ALL=C wc -c < 'gnu/sharutils-4.13.4/src/shar.c'` -ne 60556 && \
${echo} "restoration warning: size of 'gnu/sharutils-4.13.4/src/shar.c' is not 60556"
However, the function in question, emit_char_ct_validation, uses the
command constructed from cct_cmd[] also for running wc locally, via
popen, at shar creation time -- that's how it produces the
verification value (60556 in the example above). My changes touched
only the local invocation, not the verification command that gets
recorded in the shar and is supposed to run in a Unixy shell.
This problem is exacerbated by the fact that all the file names need
to have their backslash directory separators converted to Unix-style
forward slashes before they get to shar-creation code. (This is in a
part of my changes I didn't show.) This is because file names
recorded in the shar _must_ be Posix compatible. So even if the
quoting problem would somehow be solved (e.g., by adding a special
style to the quotearg module in gnulib), redirection will still not
work, because the Windows shell which is run by popen does not
understand forward slashes in file names.
So I think a Windows-specific local command for running wc at shar
creation time is the best compromise. Since the quoted strings are
file names, and Windows doesn't allow quote characters in file names,
simply enclosing each name in quotes is enough.
> > 11.shar.c uses /dev/null in freopen, but does not use the gnulib
> > freopen module, which makes /dev/null work on MS-Windows.
>
> Isn't there a POSIX layer that can be added to windows?
on the Windows runtime libraries.
In any case, gnulib has the freopen module which solves the /dev/null
problem. It just needs to be added to the package.
> > 1. configure errors out:
> >
> > configure: error: you must have sys_mman.h on your system
>
> > I don't understand why configure insists on having sys/mman.h and
> > sys/wait.h, because the code has appropriate fallbacks for when
> > they are not present.
>
> I think the correct fix is to do the gnulib futzing and then
> set ac_cv_header_sys_wait_h and ac_cv_header_sys_mman_h to "yes".
> The libopts stuff should follow that. (I don't think stuffing
> gnulib stuff into libopts would work so well.)
code for when sys/mman.h and mmap are missing -- see text_mmap.c,
which can cope with HAVE_MMAP being undefined. The only problem is
that libopts/m4/libopts.m4 explicitly insists on some headers being
present:
for f in sys_types sys_mman sys_param sys_stat sys_wait \
string errno stdlib memory setjmp
do eval as_ac_var=\${ac_cv_header_${f}_h}
test "X${as_ac_var}" = Xyes || {
]AC_MSG_ERROR([you must have ${f}.h on your system])[
done
Simply removing sys_mman and sys_wait from this list solves the
problem.
> http://autogen.sourceforge.net/data/
>
> has both a "pre2" sharutils and the autogen for creating it.
Eli Zaretskii
Apr 8, 2013, 11:52:43 AM4/8/13
to Bruce Korb, bug-gn...@gnu.org
I think I found another bug: the -T option doesn't seem to work. (It
is easy for me to see that because any non-text file causes my 'shar'
to bail out because 'fork' always fails.)
This seems to happen because configure_shar forces MIXED_UUENCODE
option unless the -C (COMPACTER) option was given:
if (! HAVE_OPT(COMPACTOR))
SET_OPT_MIXED_UUENCODE;
That in effect overrides any of the mixed-uuencode options given on
the command line, including -T.
To fix that, I modified the condition to also include an explicit
MIXED_UUENCODE option:
if (! HAVE_OPT(COMPACTOR) && ! HAVE_OPT(MIXED_UUENCODE))
SET_OPT_MIXED_UUENCODE;
As an aside, I found the documentation of the "-C none" option
confusing. The docs seem to imply that "-C none" will _also_ have the
effect of preventing uuencode of non-text files (note the second
sentence):
Specifying the compactor "none" will then disable file compres-
sion, even for non-text files. Any other compactor will force
uuencoding of files, and the recipient must have uudecode to
unpack the archive. (Compressed files are never processed as
plain text.)
However, as a matter of fact, non-text files are still being run
through uuencode, even under "-C none", because this option only
controls the compression, not whether stuff will be uuencoded -- the
latter is just a _side_effect_ of compression. I realized this only
by reading the code which implements "-C none".
I hope it will be possible to make this part of the docs less
confusing.
P.S. I must confess that the code generated by libopts is very hard
to penetrate, when one tries to find a bug. Those multiple layers of
complicated macros, similarly named options, and tricky control flow
makes it very hard to find which code does what. Some options are
acted upon explicitly in the program sources, others get set in the
bowels of libopts. E.g., to find the root cause of the -T snafu, I
needed to put a watchpoint on the corresponding element of the options
structure -- a clear sign that I had no idea where that option gets
acted upon. Just a rant...
Thanks.
is easy for me to see that because any non-text file causes my 'shar'
to bail out because 'fork' always fails.)
This seems to happen because configure_shar forces MIXED_UUENCODE
option unless the -C (COMPACTER) option was given:
if (! HAVE_OPT(COMPACTOR))
SET_OPT_MIXED_UUENCODE;
That in effect overrides any of the mixed-uuencode options given on
the command line, including -T.
To fix that, I modified the condition to also include an explicit
MIXED_UUENCODE option:
if (! HAVE_OPT(COMPACTOR) && ! HAVE_OPT(MIXED_UUENCODE))
SET_OPT_MIXED_UUENCODE;
As an aside, I found the documentation of the "-C none" option
confusing. The docs seem to imply that "-C none" will _also_ have the
effect of preventing uuencode of non-text files (note the second
sentence):
Specifying the compactor "none" will then disable file compres-
sion, even for non-text files. Any other compactor will force
uuencoding of files, and the recipient must have uudecode to
unpack the archive. (Compressed files are never processed as
plain text.)
However, as a matter of fact, non-text files are still being run
through uuencode, even under "-C none", because this option only
controls the compression, not whether stuff will be uuencoded -- the
latter is just a _side_effect_ of compression. I realized this only
by reading the code which implements "-C none".
I hope it will be possible to make this part of the docs less
confusing.
P.S. I must confess that the code generated by libopts is very hard
to penetrate, when one tries to find a bug. Those multiple layers of
complicated macros, similarly named options, and tricky control flow
makes it very hard to find which code does what. Some options are
acted upon explicitly in the program sources, others get set in the
bowels of libopts. E.g., to find the root cause of the -T snafu, I
needed to put a watchpoint on the corresponding element of the options
structure -- a clear sign that I had no idea where that option gets
acted upon. Just a rant...
Thanks.
Eli Zaretskii
Apr 8, 2013, 11:56:41 AM4/8/13
to sch...@linux-m68k.org, bug-gn...@gnu.org
> Date: Sun, 07 Apr 2013 20:55:56 +0300
> From: Eli Zaretskii <el...@gnu.org>
> Cc: bug-gn...@gnu.org
Windows headers do define TZNAME_MAX, with a value of 10. But that
value is clearly insufficient, because strftime on Windows adds
something like "Daylight Time" to the name of the zone, which enlarges
it quite a bit.
> From: Eli Zaretskii <el...@gnu.org>
> Cc: bug-gn...@gnu.org
>
> > It is even wrong for Unix, where timezone names can have as much as
> > TZNAME_MAX (typically 6) characters, and a couple of current zones in
> > the Olsen database have more than 4 characters, let alone a lot of
> > historical ones.
>
> Thanks, I didn't know about TZNAME_MAX. (It doesn't exist on Windows,
> FWIW.)
For the record: the last sentence is incorrect -- latest versions of
> > It is even wrong for Unix, where timezone names can have as much as
> > TZNAME_MAX (typically 6) characters, and a couple of current zones in
> > the Olsen database have more than 4 characters, let alone a lot of
> > historical ones.
>
> Thanks, I didn't know about TZNAME_MAX. (It doesn't exist on Windows,
> FWIW.)
Windows headers do define TZNAME_MAX, with a value of 10. But that
value is clearly insufficient, because strftime on Windows adds
something like "Daylight Time" to the name of the zone, which enlarges
it quite a bit.
Bruce Korb
Apr 8, 2013, 1:07:32 PM4/8/13
to Eli Zaretskii, bug-gn...@gnu.org, sch...@linux-m68k.org
On 04/08/13 08:56, Eli Zaretskii wrote:
>> Thanks, I didn't know about TZNAME_MAX. (It doesn't exist on Windows,
>> FWIW.)
>
> For the record: the last sentence is incorrect -- latest versions of
> Windows headers do define TZNAME_MAX, with a value of 10. But that
> value is clearly insufficient, because strftime on Windows adds
> something like "Daylight Time" to the name of the zone, which enlarges
> it quite a bit.
I settled on 60. Is that enough to cover any conceivable result?
>> Thanks, I didn't know about TZNAME_MAX. (It doesn't exist on Windows,
>> FWIW.)
>
> For the record: the last sentence is incorrect -- latest versions of
> Windows headers do define TZNAME_MAX, with a value of 10. But that
> value is clearly insufficient, because strftime on Windows adds
> something like "Daylight Time" to the name of the zone, which enlarges
> it quite a bit.
Unfortunately, the result is "0" to indicate failure, yielding no
information about how much space I ought to have had. I could do
the loop over and over until something was "big enough", but instead
if it takes more than 60, then you just don't get the submitter name.
Go live someplace that uses a smaller time zone name. ;)
Eli Zaretskii
Apr 8, 2013, 1:23:01 PM4/8/13
to Bruce Korb, bug-gn...@gnu.org, sch...@linux-m68k.org
> Date: Mon, 08 Apr 2013 10:07:32 -0700
> From: Bruce Korb <bk...@gnu.org>
> CC: sch...@linux-m68k.org, bug-gn...@gnu.org
> Unfortunately, the result is "0" to indicate failure, yielding no
> information about how much space I ought to have had. I could do
> the loop over and over until something was "big enough", but instead
> if it takes more than 60, then you just don't get the submitter name.
> Go live someplace that uses a smaller time zone name. ;)
It might be a good alternative to fall back on %z, which has a
predictable fixed limit, if %Z fails.
> From: Bruce Korb <bk...@gnu.org>
> CC: sch...@linux-m68k.org, bug-gn...@gnu.org
>
> On 04/08/13 08:56, Eli Zaretskii wrote:
> >> Thanks, I didn't know about TZNAME_MAX. (It doesn't exist on Windows,
> >> FWIW.)
> >
> > For the record: the last sentence is incorrect -- latest versions of
> > Windows headers do define TZNAME_MAX, with a value of 10. But that
> > value is clearly insufficient, because strftime on Windows adds
> > something like "Daylight Time" to the name of the zone, which enlarges
> > it quite a bit.
>
> I settled on 60. Is that enough to cover any conceivable result?
Yes, I think so.
> On 04/08/13 08:56, Eli Zaretskii wrote:
> >> Thanks, I didn't know about TZNAME_MAX. (It doesn't exist on Windows,
> >> FWIW.)
> >
> > For the record: the last sentence is incorrect -- latest versions of
> > Windows headers do define TZNAME_MAX, with a value of 10. But that
> > value is clearly insufficient, because strftime on Windows adds
> > something like "Daylight Time" to the name of the zone, which enlarges
> > it quite a bit.
>
> I settled on 60. Is that enough to cover any conceivable result?
> Unfortunately, the result is "0" to indicate failure, yielding no
> information about how much space I ought to have had. I could do
> the loop over and over until something was "big enough", but instead
> if it takes more than 60, then you just don't get the submitter name.
> Go live someplace that uses a smaller time zone name. ;)
predictable fixed limit, if %Z fails.
Bruce Korb
Apr 8, 2013, 5:09:49 PM4/8/13
to Eli Zaretskii, bug-gn...@gnu.org
On 04/08/13 08:52, Eli Zaretskii wrote:
> I think I found another bug: the -T option doesn't seem to work.
[...]
> I think I found another bug: the -T option doesn't seem to work.
> To fix that, I modified the condition to also include an explicit
> MIXED_UUENCODE option:
>
> if (! HAVE_OPT(COMPACTOR) && ! HAVE_OPT(MIXED_UUENCODE))
> SET_OPT_MIXED_UUENCODE;
In the end, that code isn't even needed. If no mixed-uuencode
> MIXED_UUENCODE option:
>
> if (! HAVE_OPT(COMPACTOR) && ! HAVE_OPT(MIXED_UUENCODE))
> SET_OPT_MIXED_UUENCODE;
option has been specified, then the default is "mixed" anyway.
The state of WHICH_OPT_MIXED_UUENCODE detects any of the four
states ("0" being the default/unspecified state):
> switch (WHICH_OPT_MIXED_UUENCODE) {
> case VALUE_OPT_TEXT_FILES: return 0;
> case VALUE_OPT_UUENCODE: return 1;
> case VALUE_OPT_MIXED_UUENCODE:
> default: break; }
> As an aside, I found the documentation of the "-C none" option
> confusing. The docs seem to imply that "-C none" will _also_ have the
> effect of preventing uuencode of non-text files (note the second
> sentence):
It says nothing about the decision to encode. The default
is mixed
> Specifying the compactor "none" will then disable file compres-
> sion, even for non-text files. Any other compactor will force
> uuencoding of files, and the recipient must have uudecode to
> unpack the archive. (Compressed files are never processed as
> plain text.)
says only that if you compress, then you uuencode. If you compress all files,
then you uuencode all files, too. How's this:
> Specifying the compactor "@samp{none}" will disable file compression.
> Any other compactor will force uuencoding of files, and the recipient
> must have @command{uudecode} to unpack the archive. (Compressed files
> are never processed as plain text.)
Immediately above the switch statement above is this code:
> if (cmpr_state != NULL)
> return 1; // compression always implies encoding
> I hope it will be possible to make this part of the docs less
> confusing.
BTW,:
> + char *command = alloca (sizeof(local_cmd) + strlen (local_name) + 2);
> + sprintf (command, "%s \"%s\"", local_cmd, local_name);
OK, I think _my_ eyeballs jumped. Sorry. I think this is adequate for this use here,
> + sprintf (command, "%s \"%s\"", local_cmd, local_name);
but should probably be derived from another variation on the quoting style names:
> enum quoting_style const quoting_style_vals[] =
> {
> literal_quoting_style,
> shell_quoting_style,
> shell_always_quoting_style,
> c_quoting_style,
> c_maybe_quoting_style,
> escape_quoting_style,
> locale_quoting_style,
> clocale_quoting_style
> };
I was using "shell_quoting_style" and we need "windows_cmd_quoting_style".
On 04/08/13 10:23, Eli Zaretskii wrote:
> It might be a good alternative to fall back on %z, which has a
> predictable fixed limit, if %Z fails.
{
static char ftime_fmt[] = "%Y-%m-%d %H:%M %Z";
/*
* All fields are two characters, except %Y is four and
* we'll drop back to "%z". We'll give up if that fails.
*/
char buffer[sizeof (ftime_fmt) + 64];
char buffer[sizeof (ftime_fmt) + 64];
time_t now;
struct tm * local_time;
time (&now);
local_time = localtime (&now);
struct tm * local_time;
time (&now);
local_time = localtime (&now);
{
size_t l =
strftime (buffer, sizeof (buffer) - 1, ftime_fmt, local_time);
if (l == 0)
size_t l =
strftime (buffer, sizeof (buffer) - 1, ftime_fmt, local_time);
{
ftime_fmt[sizeof(ftime_fmt) - 2] = 'z';
Eli Zaretskii
Apr 8, 2013, 10:57:00 PM4/8/13
to Bruce Korb, bug-gn...@gnu.org
> Date: Mon, 08 Apr 2013 14:09:49 -0700
> From: Bruce Korb <bk...@gnu.org>
> CC: bug-gn...@gnu.org
> > Specifying the compactor "@samp{none}" will disable file compression.
> > Any other compactor will force uuencoding of files, and the recipient
> > must have @command{uudecode} to unpack the archive. (Compressed files
> > are never processed as plain text.)
I would suggest
Specifying the compactor "@samp{none}" will disable file compression.
Compression forces uuencoding of compressed files, and the recipient
> > + char *command = alloca (sizeof(local_cmd) + strlen (local_name) + 2);
> > + sprintf (command, "%s \"%s\"", local_cmd, local_name);
>
> OK, I think _my_ eyeballs jumped. Sorry. I think this is adequate for this use here,
> but should probably be derived from another variation on the quoting style names:
But then you'd need to pass yet another argument to that function. It
currently gets the file with and without quoting. Now you will need a
3rd argument, quoted as appropriate for local commands. That's
doable, of course.
> {
> static char ftime_fmt[] = "%Y-%m-%d %H:%M %Z";
>
> /*
> * All fields are two characters, except %Y is four and
> * %Z may be up to 30 (?!?!). Anyway, if that still fails,
> * we'll drop back to "%z". We'll give up if that fails.
> */
> char buffer[sizeof (ftime_fmt) + 64];
> time_t now;
> struct tm * local_time;
> time (&now);
> local_time = localtime (&now);
> {
> size_t l =
> strftime (buffer, sizeof (buffer) - 1, ftime_fmt, local_time);
> if (l == 0)
> {
> ftime_fmt[sizeof(ftime_fmt) - 2] = 'z';
> l = strftime (buffer, sizeof (buffer) - 1, ftime_fmt, local_time);
> }
> if (l > 0)
> fprintf (fp, made_on_comment_z, buffer, OPT_ARG(SUBMITTER));
> }
> }
ftime_fmt is a static variable. Doesn't replacing a character in it
make that change permanent for all the following uses? Or are you
sure this array will never be used more than once in the same run?
> From: Bruce Korb <bk...@gnu.org>
> CC: bug-gn...@gnu.org
>
> > Specifying the compactor "none" will then disable file compres-
> > sion, even for non-text files. Any other compactor will force
> > uuencoding of files, and the recipient must have uudecode to
> > unpack the archive. (Compressed files are never processed as
> > plain text.)
>
> The operative phrase here is "Compressed files are never processed as plain text."
IMO, it's not a good idea to have the operative phrase in parentheses ;-)
> > Specifying the compactor "none" will then disable file compres-
> > sion, even for non-text files. Any other compactor will force
> > uuencoding of files, and the recipient must have uudecode to
> > unpack the archive. (Compressed files are never processed as
> > plain text.)
>
> The operative phrase here is "Compressed files are never processed as plain text."
> > Specifying the compactor "@samp{none}" will disable file compression.
> > Any other compactor will force uuencoding of files, and the recipient
> > must have @command{uudecode} to unpack the archive. (Compressed files
> > are never processed as plain text.)
Specifying the compactor "@samp{none}" will disable file compression.
must have @command{uudecode} to unpack the archive. (Compressed files
are never processed as plain text.)
> Immediately above the switch statement above is this code:
>
> > if (cmpr_state != NULL)
> > return 1; // compression always implies encoding
Yes, that's how I understood what was going on.
are never processed as plain text.)
> Immediately above the switch statement above is this code:
>
> > if (cmpr_state != NULL)
> > return 1; // compression always implies encoding
> > + char *command = alloca (sizeof(local_cmd) + strlen (local_name) + 2);
> > + sprintf (command, "%s \"%s\"", local_cmd, local_name);
>
> OK, I think _my_ eyeballs jumped. Sorry. I think this is adequate for this use here,
> but should probably be derived from another variation on the quoting style names:
currently gets the file with and without quoting. Now you will need a
3rd argument, quoted as appropriate for local commands. That's
doable, of course.
> {
> static char ftime_fmt[] = "%Y-%m-%d %H:%M %Z";
>
> /*
> * All fields are two characters, except %Y is four and
> * %Z may be up to 30 (?!?!). Anyway, if that still fails,
> * we'll drop back to "%z". We'll give up if that fails.
> */
> char buffer[sizeof (ftime_fmt) + 64];
> time_t now;
> struct tm * local_time;
> time (&now);
> local_time = localtime (&now);
> {
> size_t l =
> strftime (buffer, sizeof (buffer) - 1, ftime_fmt, local_time);
> if (l == 0)
> {
> ftime_fmt[sizeof(ftime_fmt) - 2] = 'z';
> l = strftime (buffer, sizeof (buffer) - 1, ftime_fmt, local_time);
> }
> if (l > 0)
> fprintf (fp, made_on_comment_z, buffer, OPT_ARG(SUBMITTER));
> }
> }
make that change permanent for all the following uses? Or are you
sure this array will never be used more than once in the same run?
Eli Zaretskii
Apr 9, 2013, 12:21:38 PM4/9/13
to bk...@gnu.org, bug-gn...@gnu.org
> Date: Mon, 08 Apr 2013 18:35:45 +0300
> From: Eli Zaretskii <el...@gnu.org>
> Cc: bug-g...@gnu.org, bug-gn...@gnu.org
>
> > Date: Sun, 07 Apr 2013 13:46:00 -0700
> > From: Bruce Korb <bk...@gnu.org>
them. So the changes below resurrect the full strength of 'shar' even
when 'fork' is not available. The trick is to use shell pipes
instead.
With these changes, all the tests in v4.13.4 pass, with a single
exception: shar-1, and that failure is not a real one:
+ ./shar-1
*** shar-1-7252.sample Tue Apr 9 13:17:07 2013
--- shar-1-7252.outf Tue Apr 9 13:17:07 2013
***************
*** 12,18 ****
# This shar contains:
# length mode name
# ------ ---------- ------------------------------------------
! # 51 -rw-r--r-- shar-1.in
#
MD5SUM=${MD5SUM-md5sum}
f=`${MD5SUM} --version | egrep '^md5sum .*(core|text)utils'`
--- 12,18 ----
# This shar contains:
# length mode name
# ------ ---------- ------------------------------------------
! # 51 -rw-rw-rw- shar-1.in
#
MD5SUM=${MD5SUM-md5sum}
f=`${MD5SUM} --version | egrep '^md5sum .*(core|text)utils'`
***************
*** 125,131 ****
SHAR_EOF
(set <date> 'shar-1.in'
eval "${shar_touch}") && \
! chmod 0644 'shar-1.in'
if test $? -ne 0
then ${echo} "restore of shar-1.in failed"
fi
--- 125,131 ----
SHAR_EOF
(set <date> 'shar-1.in'
eval "${shar_touch}") && \
! chmod 0666 'shar-1.in'
if test $? -ne 0
then ${echo} "restore of shar-1.in failed"
fi
IOW, it "fails" because mode bits for group and other do not match,
which is expected on Windows.
Here are the changes:
--- src/shar.c~3 2013-04-09 13:37:23.287005000 +0300
+++ src/shar.c 2013-04-09 13:45:18.562796500 +0300
@@ -173,7 +173,20 @@ typedef struct {
compact_state_t gzip_compaction = {
.cmpr_name = "gzip",
+#if HAVE_WORKING_FORK
.cmpr_cmd_fmt = "gzip -c -%u < %s",
+#else
+ /* When 'fork' is unavailable, we use shell pipes instead, and
+ delegate the encoding to uuencode. MinGW needs special quoting,
+ while Posix systems use a quoted file name produced by quotearg.
+ Also, avoid redirection for MinGW, since Windows shells cannot
+ redirect from files with Unix-style forward slashes. */
+#ifdef __MINGW32__
+ .cmpr_cmd_fmt = "gzip -c -%u \"%s\" | uuencode \"%s\"",
+#else
+ .cmpr_cmd_fmt = "gzip -c -%u < %s | uuencode %s",
+#endif
+#endif
.cmpr_title = "gzipped",
.cmpr_mode = "gzi",
.cmpr_unpack = "gzip -d < ${lock_dir}/gzi > %s && \\\n",
@@ -182,7 +195,15 @@ compact_state_t gzip_compaction = {
compact_state_t xz_compaction = {
.cmpr_name = "xz",
+#if HAVE_WORKING_FORK
.cmpr_cmd_fmt = "xz -z -c -%u < %s",
+#else
+#ifdef __MINGW32__
+ .cmpr_cmd_fmt = "xz -z -c -%u \"%s\" | uuencode \"%s\"",
+#else
+ .cmpr_cmd_fmt = "xz -z -c -%u < %s | uuencode %s",
+#endif
+#endif
.cmpr_title = "xz-compressed",
.cmpr_mode = "xzi",
.cmpr_unpack = "xz --decompress < ${lock_dir}/xzi > %s && \\\n",
@@ -191,7 +212,15 @@ compact_state_t xz_compaction = {
compact_state_t bzip2_compaction = {
.cmpr_name = "bzip2",
+#if HAVE_WORKING_FORK
.cmpr_cmd_fmt = "bzip2 -z -c -%u < %s",
+#else
+#ifdef __MINGW32__
+ .cmpr_cmd_fmt = "bzip2 -z -c -%u \"%s\" | uuencode \"%s\"",
+#else
+ .cmpr_cmd_fmt = "bzip2 -z -c -%u < %s | uuencode %s",
+#endif
+#endif
.cmpr_title = "bzipped",
.cmpr_mode = "bzi",
.cmpr_unpack = "bzip2 -d < ${lock_dir}/bzi > %s && \\\n",
@@ -201,7 +230,15 @@ compact_state_t bzip2_compaction = {
#ifdef HAVE_COMPRESS
compact_state_t compress_compaction = {
.cmpr_name = "compress",
+#if HAVE_WORKING_FORK
.cmpr_cmd_fmt = "compress -b%u < %s",
+#else
+#ifdef __MINGW32__
+ .cmpr_cmd_fmt = "compress -b%u \"%s\" | uuencode \"%s\"",
+#else
+ .cmpr_cmd_fmt = "compress -b%u < %s | uuencode %s",
+#endif
+#endif
.cmpr_title = "compressed",
.cmpr_mode = "cmp",
.cmpr_unpack = "compress -d < ${lock_dir}/cmp > %s && \\\n",
@@ -1270,6 +1307,8 @@ file_needs_encoding (char const * fname)
#undef BYTE_IS_BINARY
}
+#if HAVE_WORKING_FORK
+
static void
encode_file_to_pipe (
int out_fd,
@@ -1315,6 +1354,71 @@ encode_file_to_pipe (
exit (EXIT_SUCCESS);
}
+#else /* !HAVE_WORKING_FORK */
+
+static FILE *
+compress_and_encode_file (
+ const char * local_name,
+ const char * q_local_name,
+ const char * restore_name)
+{
+ /* Start writing the pipe with encodes. */
+
+ FILE * in_fp;
+ char * cmdline;
+ /* A command to use for encoding an uncompressed text file. */
+#ifdef __MINGW32__
+ static char uu_cmd_fmt[] = "uuencode \"%s\" \"%s\"";
+#else
+ static char uu_cmd_fmt[] = "uuencode %s %s";
+#endif
+
+ if (cmpr_state != NULL)
+ {
+ /* 2 * strlen (restore_name) to allow for quoting. */
+ cmdline = alloca (strlen (q_local_name) + 2 * strlen (restore_name)
+ + strlen (cmpr_state->cmpr_cmd_fmt)
+ /* 10 for compression level, 1 for null. */
+ + 10 + 1);
+ /* See commentary for gzip_compaction above, for the reasons why
+ MinGW uses a slightly different command in the pipe. */
+#ifdef __MINGW32__
+ sprintf (cmdline, cmpr_state->cmpr_cmd_fmt,
+ cmpr_state->cmpr_level, local_name, restore_name);
+#else
+ sprintf (cmdline, cmpr_state->cmpr_cmd_fmt,
+ cmpr_state->cmpr_level, q_local_name,
+ quotearg_n_style (QUOT_ID_RNAME, shell_always_quoting_style,
+ restore_name));
+#endif
+ }
+ else
+ {
+ cmdline = alloca (strlen (q_local_name) + 2 * strlen (restore_name)
+ + strlen (uu_cmd_fmt)
+ /* 10 for compression level, 1 for null. */
+ + 10 + 1);
+#ifdef __MINGW32__
+ sprintf (cmdline, uu_cmd_fmt, local_name, restore_name);
+#else
+ sprintf (cmdline, uu_cmd_fmt, q_local_name,
+ quotearg_n_style (QUOT_ID_RNAME, shell_always_quoting_style,
+ restore_name));
+#endif
+ }
+ /* Don't use freadonly_mode because it might be "rb", while we need
+ text-mode read here, because we will be reading pure text from
+ uuencode, and we want to drop any CR characters from the CRLF
+ line endings, when we write the result into the shar. */
+ in_fp = popen (cmdline, "r");
+
+ if (in_fp == NULL)
+ fserr (SHAR_EXIT_FAILED, "popen", cmdline);
+
+ return in_fp;
+}
+#endif /* !HAVE_WORKING_FORK */
+
#ifdef __MINGW32__
int
isatty (int fd)
@@ -1330,7 +1434,8 @@ open_shar_input (
const char * restore_name,
const char * q_restore_name,
const char ** file_type_p,
- const char ** file_type_remote_p)
+ const char ** file_type_remote_p,
+ int *pipe_p)
{
FILE * infp;
@@ -1348,6 +1453,7 @@ open_shar_input (
infp = fopen (local_name, freadonly_mode);
if (infp == NULL)
fserr (SHAR_EXIT_FAILED, "fopen", local_name);
+ *pipe_p = 0;
}
else
{
@@ -1356,6 +1462,7 @@ open_shar_input (
*file_type_p = cmpr_state ? cmpr_state->cmpr_title : _("text");
*file_type_remote_p = cmpr_state ? cmpr_state->cmpr_title : _("(text)");
+#if HAVE_WORKING_FORK
/* Fork a uuencode process. */
if (pipe (pipex) < 0)
@@ -1381,6 +1488,10 @@ open_shar_input (
if (infp == NULL)
fserr (SHAR_EXIT_FAILED, "fdopen", _("pipe[1]"));
}
+#else /* !HAVE_WORKING_FORK */
+ infp = compress_and_encode_file (local_name, q_local_name, restore_name);
+#endif
+ *pipe_p = 1;
}
return infp;
@@ -1579,7 +1690,7 @@ process_shar_input (FILE * input, off_t
static int
start_sharing_file (char const ** lnameq_p, char const ** rnameq_p,
- FILE ** fpp, off_t * size_left_p)
+ FILE ** fpp, off_t * size_left_p, int *pipe_p)
{
char const * lname = *lnameq_p;
char const * rname = *rnameq_p;
@@ -1645,7 +1756,7 @@ start_sharing_file (char const ** lnameq
else
{
*fpp = open_shar_input (lname, *lnameq_p, rname, *rnameq_p,
- &file_type, &file_type_remote);
+ &file_type, &file_type_remote, pipe_p);
if (*fpp == NULL)
return 0;
}
@@ -1777,8 +1888,9 @@ shar (const char * lname, const char * r
int split_flag = 0; /* file split flag */
char const * lname_q = lname;
char const * rname_q = rname;
+ int pipe_p; /* opened directly or via a pipe */
- if (! start_sharing_file (&lname_q, &rname_q, &input, &size_left))
+ if (! start_sharing_file (&lname_q, &rname_q, &input, &size_left, &pipe_p))
return SHAR_EXIT_FAILED;
if (struct_stat.st_size == 0)
@@ -1815,9 +1927,18 @@ shar (const char * lname, const char * r
process_shar_input (input, &size_left, &split_flag, rname, rname_q);
- fclose (input);
- while (wait (NULL) >= 0)
- ;
+ if (!pipe_p)
+ fclose (input);
+ else
+ {
+#if HAVE_WORKING_FORK
+ while (wait (NULL) >= 0)
+ ;
+#else
+ if (pclose (input))
+ fserr (SHAR_EXIT_FAILED, _("call"), "pclose");
+ }
+#endif
fprintf (output, "%s\n", OPT_ARG(HERE_DELIMITER));
if (split_flag && ! HAVE_OPT(QUIET_UNSHAR))
@@ -1935,6 +2056,9 @@ open_output (void)
if (! HAVE_OPT(OUTPUT_PREFIX))
{
output = stdout;
+#ifdef __MINGW32__
+ _setmode (fileno (stdout) , _O_BINARY);
+#endif
return;
}
> From: Eli Zaretskii <el...@gnu.org>
> Cc: bug-g...@gnu.org, bug-gn...@gnu.org
>
> > Date: Sun, 07 Apr 2013 13:46:00 -0700
> > From: Bruce Korb <bk...@gnu.org>
> > CC: bug-gn...@gnu.org, bug-g...@gnu.org,
> > Andreas Schwab <sch...@linux-m68k.org>
> >
> > > For the other errors: I wrote emulations for fork (always fails),
> > > wait (always fails) and pipe (calls _pipe), and also add getuid,
> > > getpwnam and getgrnam to idcache.c. However, I think gnulib has
> > > replacements for at least some of these functions.
> >
> > Not that I can find.
> > > $ list=$(find * -type f)
> > > $ fgrep getuid $list
> > > $ fgrep getpwnam $list
> > > $ fgrep getgrnam $list
> > > $ echo "$list"|egrep "getuid|getpwnam|getgrnam"
> > > $
>
> I meant fork, wait (waitpid in gnulib), and pipe.
Anyway, I hate it when programs lose features as result of porting
> > Andreas Schwab <sch...@linux-m68k.org>
> >
> > > For the other errors: I wrote emulations for fork (always fails),
> > > wait (always fails) and pipe (calls _pipe), and also add getuid,
> > > getpwnam and getgrnam to idcache.c. However, I think gnulib has
> > > replacements for at least some of these functions.
> >
> > Not that I can find.
> > > $ list=$(find * -type f)
> > > $ fgrep getuid $list
> > > $ fgrep getpwnam $list
> > > $ fgrep getgrnam $list
> > > $ echo "$list"|egrep "getuid|getpwnam|getgrnam"
> > > $
>
> I meant fork, wait (waitpid in gnulib), and pipe.
them. So the changes below resurrect the full strength of 'shar' even
when 'fork' is not available. The trick is to use shell pipes
instead.
With these changes, all the tests in v4.13.4 pass, with a single
exception: shar-1, and that failure is not a real one:
+ ./shar-1
*** shar-1-7252.sample Tue Apr 9 13:17:07 2013
--- shar-1-7252.outf Tue Apr 9 13:17:07 2013
***************
*** 12,18 ****
# This shar contains:
# length mode name
# ------ ---------- ------------------------------------------
! # 51 -rw-r--r-- shar-1.in
#
MD5SUM=${MD5SUM-md5sum}
f=`${MD5SUM} --version | egrep '^md5sum .*(core|text)utils'`
--- 12,18 ----
# This shar contains:
# length mode name
# ------ ---------- ------------------------------------------
! # 51 -rw-rw-rw- shar-1.in
#
MD5SUM=${MD5SUM-md5sum}
f=`${MD5SUM} --version | egrep '^md5sum .*(core|text)utils'`
***************
*** 125,131 ****
SHAR_EOF
(set <date> 'shar-1.in'
eval "${shar_touch}") && \
! chmod 0644 'shar-1.in'
if test $? -ne 0
then ${echo} "restore of shar-1.in failed"
fi
--- 125,131 ----
SHAR_EOF
(set <date> 'shar-1.in'
eval "${shar_touch}") && \
! chmod 0666 'shar-1.in'
if test $? -ne 0
then ${echo} "restore of shar-1.in failed"
fi
IOW, it "fails" because mode bits for group and other do not match,
which is expected on Windows.
Here are the changes:
--- src/shar.c~3 2013-04-09 13:37:23.287005000 +0300
+++ src/shar.c 2013-04-09 13:45:18.562796500 +0300
@@ -173,7 +173,20 @@ typedef struct {
compact_state_t gzip_compaction = {
.cmpr_name = "gzip",
+#if HAVE_WORKING_FORK
.cmpr_cmd_fmt = "gzip -c -%u < %s",
+#else
+ /* When 'fork' is unavailable, we use shell pipes instead, and
+ delegate the encoding to uuencode. MinGW needs special quoting,
+ while Posix systems use a quoted file name produced by quotearg.
+ Also, avoid redirection for MinGW, since Windows shells cannot
+ redirect from files with Unix-style forward slashes. */
+#ifdef __MINGW32__
+ .cmpr_cmd_fmt = "gzip -c -%u \"%s\" | uuencode \"%s\"",
+#else
+ .cmpr_cmd_fmt = "gzip -c -%u < %s | uuencode %s",
+#endif
+#endif
.cmpr_title = "gzipped",
.cmpr_mode = "gzi",
.cmpr_unpack = "gzip -d < ${lock_dir}/gzi > %s && \\\n",
@@ -182,7 +195,15 @@ compact_state_t gzip_compaction = {
compact_state_t xz_compaction = {
.cmpr_name = "xz",
+#if HAVE_WORKING_FORK
.cmpr_cmd_fmt = "xz -z -c -%u < %s",
+#else
+#ifdef __MINGW32__
+ .cmpr_cmd_fmt = "xz -z -c -%u \"%s\" | uuencode \"%s\"",
+#else
+ .cmpr_cmd_fmt = "xz -z -c -%u < %s | uuencode %s",
+#endif
+#endif
.cmpr_title = "xz-compressed",
.cmpr_mode = "xzi",
.cmpr_unpack = "xz --decompress < ${lock_dir}/xzi > %s && \\\n",
@@ -191,7 +212,15 @@ compact_state_t xz_compaction = {
compact_state_t bzip2_compaction = {
.cmpr_name = "bzip2",
+#if HAVE_WORKING_FORK
.cmpr_cmd_fmt = "bzip2 -z -c -%u < %s",
+#else
+#ifdef __MINGW32__
+ .cmpr_cmd_fmt = "bzip2 -z -c -%u \"%s\" | uuencode \"%s\"",
+#else
+ .cmpr_cmd_fmt = "bzip2 -z -c -%u < %s | uuencode %s",
+#endif
+#endif
.cmpr_title = "bzipped",
.cmpr_mode = "bzi",
.cmpr_unpack = "bzip2 -d < ${lock_dir}/bzi > %s && \\\n",
@@ -201,7 +230,15 @@ compact_state_t bzip2_compaction = {
#ifdef HAVE_COMPRESS
compact_state_t compress_compaction = {
.cmpr_name = "compress",
+#if HAVE_WORKING_FORK
.cmpr_cmd_fmt = "compress -b%u < %s",
+#else
+#ifdef __MINGW32__
+ .cmpr_cmd_fmt = "compress -b%u \"%s\" | uuencode \"%s\"",
+#else
+ .cmpr_cmd_fmt = "compress -b%u < %s | uuencode %s",
+#endif
+#endif
.cmpr_title = "compressed",
.cmpr_mode = "cmp",
.cmpr_unpack = "compress -d < ${lock_dir}/cmp > %s && \\\n",
@@ -1270,6 +1307,8 @@ file_needs_encoding (char const * fname)
#undef BYTE_IS_BINARY
}
+#if HAVE_WORKING_FORK
+
static void
encode_file_to_pipe (
int out_fd,
@@ -1315,6 +1354,71 @@ encode_file_to_pipe (
exit (EXIT_SUCCESS);
}
+#else /* !HAVE_WORKING_FORK */
+
+static FILE *
+compress_and_encode_file (
+ const char * local_name,
+ const char * q_local_name,
+ const char * restore_name)
+{
+ /* Start writing the pipe with encodes. */
+
+ FILE * in_fp;
+ char * cmdline;
+ /* A command to use for encoding an uncompressed text file. */
+#ifdef __MINGW32__
+ static char uu_cmd_fmt[] = "uuencode \"%s\" \"%s\"";
+#else
+ static char uu_cmd_fmt[] = "uuencode %s %s";
+#endif
+
+ if (cmpr_state != NULL)
+ {
+ /* 2 * strlen (restore_name) to allow for quoting. */
+ cmdline = alloca (strlen (q_local_name) + 2 * strlen (restore_name)
+ + strlen (cmpr_state->cmpr_cmd_fmt)
+ /* 10 for compression level, 1 for null. */
+ + 10 + 1);
+ /* See commentary for gzip_compaction above, for the reasons why
+ MinGW uses a slightly different command in the pipe. */
+#ifdef __MINGW32__
+ sprintf (cmdline, cmpr_state->cmpr_cmd_fmt,
+ cmpr_state->cmpr_level, local_name, restore_name);
+#else
+ sprintf (cmdline, cmpr_state->cmpr_cmd_fmt,
+ cmpr_state->cmpr_level, q_local_name,
+ quotearg_n_style (QUOT_ID_RNAME, shell_always_quoting_style,
+ restore_name));
+#endif
+ }
+ else
+ {
+ cmdline = alloca (strlen (q_local_name) + 2 * strlen (restore_name)
+ + strlen (uu_cmd_fmt)
+ /* 10 for compression level, 1 for null. */
+ + 10 + 1);
+#ifdef __MINGW32__
+ sprintf (cmdline, uu_cmd_fmt, local_name, restore_name);
+#else
+ sprintf (cmdline, uu_cmd_fmt, q_local_name,
+ quotearg_n_style (QUOT_ID_RNAME, shell_always_quoting_style,
+ restore_name));
+#endif
+ }
+ /* Don't use freadonly_mode because it might be "rb", while we need
+ text-mode read here, because we will be reading pure text from
+ uuencode, and we want to drop any CR characters from the CRLF
+ line endings, when we write the result into the shar. */
+ in_fp = popen (cmdline, "r");
+
+ if (in_fp == NULL)
+ fserr (SHAR_EXIT_FAILED, "popen", cmdline);
+
+ return in_fp;
+}
+#endif /* !HAVE_WORKING_FORK */
+
#ifdef __MINGW32__
int
isatty (int fd)
@@ -1330,7 +1434,8 @@ open_shar_input (
const char * restore_name,
const char * q_restore_name,
const char ** file_type_p,
- const char ** file_type_remote_p)
+ const char ** file_type_remote_p,
+ int *pipe_p)
{
FILE * infp;
@@ -1348,6 +1453,7 @@ open_shar_input (
infp = fopen (local_name, freadonly_mode);
if (infp == NULL)
fserr (SHAR_EXIT_FAILED, "fopen", local_name);
+ *pipe_p = 0;
}
else
{
@@ -1356,6 +1462,7 @@ open_shar_input (
*file_type_p = cmpr_state ? cmpr_state->cmpr_title : _("text");
*file_type_remote_p = cmpr_state ? cmpr_state->cmpr_title : _("(text)");
+#if HAVE_WORKING_FORK
/* Fork a uuencode process. */
if (pipe (pipex) < 0)
@@ -1381,6 +1488,10 @@ open_shar_input (
if (infp == NULL)
fserr (SHAR_EXIT_FAILED, "fdopen", _("pipe[1]"));
}
+#else /* !HAVE_WORKING_FORK */
+ infp = compress_and_encode_file (local_name, q_local_name, restore_name);
+#endif
+ *pipe_p = 1;
}
return infp;
@@ -1579,7 +1690,7 @@ process_shar_input (FILE * input, off_t
static int
start_sharing_file (char const ** lnameq_p, char const ** rnameq_p,
- FILE ** fpp, off_t * size_left_p)
+ FILE ** fpp, off_t * size_left_p, int *pipe_p)
{
char const * lname = *lnameq_p;
char const * rname = *rnameq_p;
@@ -1645,7 +1756,7 @@ start_sharing_file (char const ** lnameq
else
{
*fpp = open_shar_input (lname, *lnameq_p, rname, *rnameq_p,
- &file_type, &file_type_remote);
+ &file_type, &file_type_remote, pipe_p);
if (*fpp == NULL)
return 0;
}
@@ -1777,8 +1888,9 @@ shar (const char * lname, const char * r
int split_flag = 0; /* file split flag */
char const * lname_q = lname;
char const * rname_q = rname;
+ int pipe_p; /* opened directly or via a pipe */
- if (! start_sharing_file (&lname_q, &rname_q, &input, &size_left))
+ if (! start_sharing_file (&lname_q, &rname_q, &input, &size_left, &pipe_p))
return SHAR_EXIT_FAILED;
if (struct_stat.st_size == 0)
@@ -1815,9 +1927,18 @@ shar (const char * lname, const char * r
process_shar_input (input, &size_left, &split_flag, rname, rname_q);
- fclose (input);
- while (wait (NULL) >= 0)
- ;
+ if (!pipe_p)
+ fclose (input);
+ else
+ {
+#if HAVE_WORKING_FORK
+ while (wait (NULL) >= 0)
+ ;
+#else
+ if (pclose (input))
+ fserr (SHAR_EXIT_FAILED, _("call"), "pclose");
+ }
+#endif
fprintf (output, "%s\n", OPT_ARG(HERE_DELIMITER));
if (split_flag && ! HAVE_OPT(QUIET_UNSHAR))
@@ -1935,6 +2056,9 @@ open_output (void)
if (! HAVE_OPT(OUTPUT_PREFIX))
{
output = stdout;
+#ifdef __MINGW32__
+ _setmode (fileno (stdout) , _O_BINARY);
+#endif
return;
}
Bruce Korb
Apr 11, 2013, 10:39:08 AM4/11/13
to Eli Zaretskii, bug-gn...@gnu.org
On 04/08/13 19:57, Eli Zaretskii wrote:
> I would suggest
>
> Specifying the compactor "@samp{none}" will disable file compression.
> Compression forces uuencoding of compressed files, and the recipient
> must have @command{uudecode} to unpack the archive. (Compressed files
> are never processed as plain text.)
Better still.
> I would suggest
>
> Specifying the compactor "@samp{none}" will disable file compression.
> Compression forces uuencoding of compressed files, and the recipient
> must have @command{uudecode} to unpack the archive. (Compressed files
> are never processed as plain text.)
>> OK, I think _my_ eyeballs jumped. Sorry. I think this is adequate for this use here,
>> but should probably be derived from another variation on the quoting style names:
>
> But then you'd need to pass yet another argument to that function.
adornment is only needed in this one place. I am actually saying the
gnulib module needs a new quoting style that would be used here.
In the end, I separated the "wc -c" string for the system() call
from the "wc -c" string for the shar script and wound up with:
> #ifndef __MINGW32__
> static char const cct_cmd[] = "LC_ALL=C wc -c < %s";
> char *command = alloca (sizeof(cct_cmd) + strlen (quoted_name) + 2);
> #else
> static char const cct_cmd[] = "set LC_ALL=C & wc -c \"%s\"";
> char *command = alloca (sizeof(cct_cmd) + strlen (local_name));
> #endif
>
> sprintf (command, cct_cmd, quoted_name);
and used a different formatting string to emit the shar text.
>> {
>> static char ftime_fmt[] = "%Y-%m-%d %H:%M %Z";
> ftime_fmt is a static variable. Doesn't replacing a character in it
> make that change permanent for all the following uses? Or are you
> sure this array will never be used more than once in the same run?
through it fails, but works with "%z" and the next time through it
would have worked with "%Z"? Then there's the consistency thing.
I think changing it to 'z' is fine as a do it once and leave it.
There's still the chance "%Z" works and then doesn't, but I'm not
going to worry over it. :) Once flipped, it stays flipped.
Thanks for reviewing this stuff. I'm leaving for a conference all
next week, so I won't actually polish this and bump out the next
rev until May.
Regards, Bruce
Bruce Korb
Apr 11, 2013, 12:01:31 PM4/11/13
to Eli Zaretskii, bug-gn...@gnu.org
On 04/09/13 09:21, Eli Zaretskii wrote:
> Anyway, I hate it when programs lose features as result of porting
> them. So the changes below resurrect the full strength of 'shar' even
> when 'fork' is not available. The trick is to use shell pipes
> instead.
>
> With these changes, all the tests in v4.13.4 pass, with a single
> exception: shar-1, and that failure is not a real one:
I've hacked tests/shar-1:
> Anyway, I hate it when programs lose features as result of porting
> them. So the changes below resurrect the full strength of 'shar' even
> when 'fork' is not available. The trick is to use shell pipes
> instead.
>
> With these changes, all the tests in v4.13.4 pass, with a single
> exception: shar-1, and that failure is not a real one:
# adjust for variations in the package string and for
# variations in Windows capabilities.
#
sed -e "s/__PACKAGE_STRING__/${PACKAGE_STRING}/" \
-e 's/-rw-rw-rw- shar-1.in/-rw-r--r-- shar-1.in/' \
-e "s/chmod 0666 'shar-1.in'/chmod 0644 'shar-1.in'/" \
$top_srcdir/tests/shar-1.ok > ${tmpfile}.sample
> Here are the changes:
I don't have time to digest this part of the patch,
other than to suggest:
#if HAVE_WORKING_FORK
#elif defined(__MINGW32__)
#else
#endif
I also haven't investigated, but I worry about adding the uuencode
command when HAVE_WORKING_FORK is zero. It changes the
printf arguments and I think the uuencode gets added on
anyhow, so wouldn't it be redundant? Dunno. Haven't looked yet.
Anyway, forcing the pipe when fork doesn't work cancels out the
--no-piping option, so this definitely needs some massaging.
Eli Zaretskii
Apr 11, 2013, 1:38:10 PM4/11/13
to Bruce Korb, bug-gn...@gnu.org
> Date: Thu, 11 Apr 2013 09:01:31 -0700
> I've hacked tests/shar-1:
Thanks.
> > Here are the changes:
>
> I don't have time to digest this part of the patch,
> other than to suggest:
>
> #if HAVE_WORKING_FORK
> #elif defined(__MINGW32__)
> #else
> #endif
It's up to you, I don't mind either way. I just didn't want to assume
that the only platform which !HAVE_WORKING_FORK is MinGW. But if
that's a safe assumption, it's fine with me.
> I also haven't investigated, but I worry about adding the uuencode
> command when HAVE_WORKING_FORK is zero. It changes the
> printf arguments and I think the uuencode gets added on
> anyhow, so wouldn't it be redundant? Dunno. Haven't looked yet.
I'm not sure I'm following. When you do have time, please tell more.
> Anyway, forcing the pipe when fork doesn't work cancels out the
> --no-piping option, so this definitely needs some massaging.
Right, forgot about that.
> I've hacked tests/shar-1:
Thanks.
> > Here are the changes:
>
> I don't have time to digest this part of the patch,
> other than to suggest:
>
> #if HAVE_WORKING_FORK
> #elif defined(__MINGW32__)
> #else
> #endif
that the only platform which !HAVE_WORKING_FORK is MinGW. But if
that's a safe assumption, it's fine with me.
> I also haven't investigated, but I worry about adding the uuencode
> command when HAVE_WORKING_FORK is zero. It changes the
> printf arguments and I think the uuencode gets added on
> anyhow, so wouldn't it be redundant? Dunno. Haven't looked yet.
> Anyway, forcing the pipe when fork doesn't work cancels out the
> --no-piping option, so this definitely needs some massaging.
0 new messages