[PATCH setup 1/4] Add Log adaptors for printf-style output

[PATCH setup 1/4] Add Log adaptors for printf-style output

[Jon TURNEY]

---
 ChangeLog       |  5 +++++
 LogSingleton.cc | 28 ++++++++++++++++++++++++++++
 LogSingleton.h  |  5 +++++
 3 files changed, 38 insertions(+)

diff --git a/ChangeLog b/ChangeLog
index 551cc94..0822701 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,8 @@
+2015-03-04  Jon TURNEY  <jon.turney@dronecode.org.uk>
+
+	* LogSingleton.cc (LogBabblePrintf, LogPlainPrintf): Add.
+	* LogSingleton.h: Ditto.
+
 2015-03-02  Jon TURNEY  <jon.turney@dronecode.org.uk>
 
 	* LogFile.cc (endEntry): Remove msg().
diff --git a/LogSingleton.cc b/LogSingleton.cc
index 387e6fe..a103a20 100644
--- a/LogSingleton.cc
+++ b/LogSingleton.cc
@@ -15,6 +15,7 @@
 
 #include "LogSingleton.h"
 #include <stdexcept>
+#include <stdarg.h>
 
 using namespace std;
 
@@ -76,3 +77,30 @@ private:
   static LogSingleton *theInstance;
 };
 #endif
+
+// Log adapators for printf-style output
+void
+LogBabblePrintf(const char *fmt, ...)
+{
+  int len;
+  char buf[2000];
+  va_list args;
+  va_start (args, fmt);
+  len = vsnprintf (buf, 2000, fmt, args);
+  if ((len > 0) && (buf[len-1] == '\n'))
+    buf[len-1] = 0;
+  Log (LOG_BABBLE) << buf << endLog;
+}
+
+void
+LogPlainPrintf(const char *fmt, ...)
+{
+  int len;
+  char buf[2000];
+  va_list args;
+  va_start (args, fmt);
+  len = vsnprintf (buf, 2000, fmt, args);
+  if ((len > 0) && (buf[len-1] == '\n'))
+    buf[len-1] = 0;
+  Log (LOG_PLAIN) << buf << endLog;
+}
diff --git a/LogSingleton.h b/LogSingleton.h
index 2c7a7c5..2fd1e36 100644
--- a/LogSingleton.h
+++ b/LogSingleton.h
@@ -57,4 +57,9 @@ extern std::ostream& endLog(std::ostream& outs);
 //extern ostream& endLog(ostream& outs);
 
 #define Log(X) (LogSingleton::GetInstance ()(X))
+
+// Log adapators for printf-style output
+void LogBabblePrintf(const char *fmt, ...);
+void LogPlainPrintf(const char *fmt, ...);
+
 #endif /* SETUP_LOGSINGLETON_H */
-- 
2.1.4

[PATCH setup 0/4] Setup logging fixes

[Jon TURNEY]

On 02/03/2015 16:32, Corinna Vinschen wrote:
> On Mar  2 13:55, Jon TURNEY wrote:
>>
>> Future work: there are some other uses of msg() to report real errors.  At the
>> moment, these are completely invisible unless you are running setup under a
>> debugger.  These should be converted to use the logger.
[...]
> Approved, under the assumption that you'll rework the other msg() calls
> as well ;)

:)

Also change some of the uses of fprintf(stderr, ...) to report errors, which doesn't get logged.

This then exposes a bit of log spam in archive_tar.cc, which is silenced.

Jon TURNEY (4):
  Add Log adaptors for printf-style output
  Remove msg() and convert to log output
  Log errors from archive_tar.cc
  Silently ignore 'x' and 'g' type tar extended headers

 ChangeLog       | 28 ++++++++++++++++++++++++++++
 LogSingleton.cc | 28 ++++++++++++++++++++++++++++
 LogSingleton.h  |  5 +++++
 archive_tar.cc  | 28 ++++++++++++----------------
 compress_xz.cc  | 22 +++++++++++-----------
 crypto.cc       |  5 +++--
 desktop.cc      | 13 ++++++-------
 gpg-packet.cc   |  5 +++--
 ini.cc          |  3 +--
 msg.cc          | 10 ----------
 msg.h           |  6 ------
 simpsock.cc     |  8 ++++----
 12 files changed, 101 insertions(+), 60 deletions(-)

-- 
2.1.4

[PATCH setup 2/4] Remove msg() and convert to log output

[Jon TURNEY]

Remove msg(), which writes output to the debugger via OutputDebugString() and
convert it's uses to log output using the printf-style log adaptors.

Examining the uses of msg(), some of these are genuine errors, which should be
logged somewhere we might have a chance to see them.  Convert those to
LOG_PLAIN.  Convert the other uses of msg() for debug output to LOG_BABBLE.
---
 ChangeLog      | 13 +++++++++++++
 compress_xz.cc | 22 +++++++++++-----------
 crypto.cc      |  5 +++--
 desktop.cc     | 13 ++++++-------
 gpg-packet.cc  |  5 +++--
 ini.cc         |  3 +--
 msg.cc         | 10 ----------
 msg.h          |  6 ------
 simpsock.cc    |  8 ++++----
 9 files changed, 41 insertions(+), 44 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 0822701..472030e 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,18 @@
 2015-03-04  Jon TURNEY  <jon.turney@dronecode.org.uk>
 
+	* msg.cc (msg): Remove.
+	* msg.h (msg): Ditto.
+	* compress_xz.cc (read, bid_xz, bid_lzma): Convert from msg() to
+	LogBabblePrintf() or LogPlainPrintf() as appropriate.
+	* crypto.cc (MESSAGE): Ditto.
+	* desktop.cc (make_link, start_menu, desktop_icon)
+	(check_desktop, check_startmenu): Ditto.
+	* gpg-packet.cc (MESSAGE): Ditto.
+	* ini.cc (do_ini_thread): Ditto.
+	* simpsock.cc (SimpleSocket): Ditto.
+
+2015-03-04  Jon TURNEY  <jon.turney@dronecode.org.uk>
+
 	* LogSingleton.cc (LogBabblePrintf, LogPlainPrintf): Add.
 	* LogSingleton.h: Ditto.
 
diff --git a/compress_xz.cc b/compress_xz.cc
index 6c9980b..1480c6c 100644
--- a/compress_xz.cc
+++ b/compress_xz.cc
@@ -18,7 +18,7 @@
  */
 
 #include "compress_xz.h"
-#include "msg.h"
+#include "LogSingleton.h"
 
 #include <stdexcept>
 using namespace std;
@@ -186,35 +186,35 @@ compress_xz::read (void *buffer, size_t len)
           case LZMA_OK: /* Decompressor made some progress. */
             break;
           case LZMA_MEM_ERROR:
-            msg ("Lzma library error: Cannot allocate memory\n");
+            LogPlainPrintf ("Lzma library error: Cannot allocate memory\n");
             this->lasterr = ENOMEM;
             return -1;
           case LZMA_MEMLIMIT_ERROR:
-            msg ("Lzma library error: Out of memory\n");
+            LogPlainPrintf ("Lzma library error: Out of memory\n");
             this->lasterr = ENOMEM;
             return -1;
           case LZMA_FORMAT_ERROR:
-            msg ("Lzma library error: format not recognized\n");
+            LogPlainPrintf ("Lzma library error: format not recognized\n");
             this->lasterr = EINVAL;
             return -1;
           case LZMA_OPTIONS_ERROR:
-            msg ("Lzma library error: Invalid options\n");
+            LogPlainPrintf ("Lzma library error: Invalid options\n");
             this->lasterr = EINVAL;
             return -1;
           case LZMA_DATA_ERROR:
-            msg ("Lzma library error: Corrupted input data\n");
+            LogPlainPrintf ("Lzma library error: Corrupted input data\n");
             this->lasterr = EINVAL;
             return -1;
           case LZMA_BUF_ERROR:
-            msg ("Lzma library error: No progress is possible\n");
+            LogPlainPrintf ("Lzma library error: No progress is possible\n");
             this->lasterr = EINVAL;
             return -1;
           case LZMA_PROG_ERROR:
-            msg ("Lzma library error: Internal error\n");
+            LogPlainPrintf ("Lzma library error: Internal error\n");
             this->lasterr = EINVAL;
             return -1;
           default:
-            msg ("Lzma decompression failed:  Unknown error %d\n", res);
+            LogPlainPrintf ("Lzma decompression failed:  Unknown error %d\n", res);
             this->lasterr = EINVAL;
             return -1;
         }
@@ -508,7 +508,7 @@ compress_xz::bid_xz (void * buffer, size_t len)
     return 0;
   bits_checked += 8;
 
-  msg ("compress_xz::bid_xz: success: %d\n", bits_checked);
+  LogBabblePrintf ("compress_xz::bid_xz: success: %d\n", bits_checked);
   return (bits_checked);
 }
 
@@ -616,7 +616,7 @@ compress_xz::bid_lzma (void * buffer, size_t len)
 
   /* TODO: The above test is still very weak.  It would be
    * good to do better. */
-  msg ("compress_xz::bid_lzma: success: %d\n", bits_checked);
+  LogBabblePrintf ("compress_xz::bid_lzma: success: %d\n", bits_checked);
   return (bits_checked);
 }
 
diff --git a/crypto.cc b/crypto.cc
index 13270a7..6ac7a54 100755
--- a/crypto.cc
+++ b/crypto.cc
@@ -27,6 +27,7 @@ static const char *cvsid =
 #include "compress.h"
 #include "gcrypt.h"
 #include "msg.h"
+#include "LogSingleton.h"
 #include "resource.h"
 #include "getopt++/StringArrayOption.h"
 #include "getopt++/BoolOption.h"
@@ -38,10 +39,10 @@ static const char *cvsid =
 
 #if CRYPTODEBUGGING
 #define ERRKIND __asm__ __volatile__ (".byte 0xcc"); note
-#define MESSAGE msg
+#define MESSAGE LogBabblePrintf
 #else  /* !CRYPTODEBUGGING */
 #define ERRKIND note
-#define MESSAGE while (0) msg
+#define MESSAGE while (0) LogBabblePrintf
 #endif /* CRYPTODEBUGGING */
 
 /*  Command-line options for specifying and controlling extra keys.  */
diff --git a/desktop.cc b/desktop.cc
index 33a9891..1add57a 100644
--- a/desktop.cc
+++ b/desktop.cc
@@ -34,7 +34,6 @@ static const char *cvsid =
 
 #include "ini.h"
 #include "resource.h"
-#include "msg.h"
 #include "state.h"
 #include "dialog.h"
 #include "mount.h"
@@ -85,7 +84,7 @@ make_link (const std::string& linkpath,
   if (_access (fname.c_str(), 0) == 0)
     return;			/* already exists */
 
-  msg ("make_link %s, %s, %s\n",
+  LogBabblePrintf ("make_link %s, %s, %s\n",
        fname.c_str(), title.c_str(), target.c_str());
 
   io_stream::mkpath_p (PATH_TO_FILE, std::string ("file://") + fname, 0);
@@ -96,7 +95,7 @@ make_link (const std::string& linkpath,
   exepath = target;
   argbuf = arg;
 
-  msg ("make_link_2 (%s, %s, %s, %s)",
+  LogBabblePrintf ("make_link_2 (%s, %s, %s, %s)",
        exepath.c_str(), argbuf.c_str(),
        icon.c_str(), fname.c_str());
   make_link_2 (exepath.c_str(), argbuf.c_str(),
@@ -116,7 +115,7 @@ start_menu (const std::string& title, const std::string& target,
 			      CSIDL_PROGRAMS, &id);
   SHGetPathFromIDList (id, path);
   strncat (path, "/Cygwin", MAX_PATH);
-  msg ("Program directory for program link: %s", path);
+  LogBabblePrintf ("Program directory for program link: %s", path);
   make_link (path, title, target, arg, iconpath);
 }
 
@@ -132,7 +131,7 @@ desktop_icon (const std::string& title, const std::string& target,
 			      issystem ? CSIDL_COMMON_DESKTOPDIRECTORY :
 			      CSIDL_DESKTOPDIRECTORY, &id);
   SHGetPathFromIDList (id, path);
-  msg ("Desktop directory for desktop link: %s", path);
+  LogBabblePrintf ("Desktop directory for desktop link: %s", path);
   make_link (path, title, target, arg, iconpath);
 }
 
@@ -287,7 +286,7 @@ check_desktop (const std::string title, const std::string target)
 			      issystem ? CSIDL_COMMON_DESKTOPDIRECTORY :
 			      CSIDL_DESKTOPDIRECTORY, &id);
   SHGetPathFromIDList (id, path);
-  msg ("Desktop directory for desktop link: %s", path);
+  LogBabblePrintf ("Desktop directory for desktop link: %s", path);
   std::string fname = std::string(path) + "/" + title + ".lnk";
 
   if (_access (fname.c_str(), 0) == 0)
@@ -307,7 +306,7 @@ check_startmenu (const std::string title, const std::string target)
 			      issystem ? CSIDL_COMMON_PROGRAMS :
 			      CSIDL_PROGRAMS, &id);
   SHGetPathFromIDList (id, path);
-  msg ("Program directory for program link: %s", path);
+  LogBabblePrintf ("Program directory for program link: %s", path);
   strcat (path, STARTMENUDIR);
   std::string fname = std::string(path) + "/" + title + ".lnk";
 
diff --git a/gpg-packet.cc b/gpg-packet.cc
index 3a3bef4..e288b0a 100755
--- a/gpg-packet.cc
+++ b/gpg-packet.cc
@@ -31,16 +31,17 @@ static const char *cvsid =
 #include "gcrypt.h"
 #include "gpg-packet.h"
 #include "msg.h"
+#include "LogSingleton.h"
 #include "resource.h"
 
 #define CRYPTODEBUGGING         (0)
 
 #if CRYPTODEBUGGING
 #define ERRKIND __asm__ __volatile__ (".byte 0xcc"); note
-#define MESSAGE msg
+#define MESSAGE LogBabblePrintf
 #else  /* !CRYPTODEBUGGING */
 #define ERRKIND note
-#define MESSAGE while (0) msg
+#define MESSAGE while (0) LogBabblePrintf
 #endif /* CRYPTODEBUGGING */
 
 #ifndef ARRAYSIZE
diff --git a/ini.cc b/ini.cc
index 7d7b917..e90f98c 100644
--- a/ini.cc
+++ b/ini.cc
@@ -36,7 +36,6 @@
 #include "state.h"
 #include "geturl.h"
 #include "dialog.h"
-#include "msg.h"
 #include "mount.h"
 #include "site.h"
 #include "find.h"
@@ -333,7 +332,7 @@ do_ini_thread (HINSTANCE h, HWND owner)
 	}
     }
 
-  msg (".ini setup_version is %s, our setup_version is %s", ini_setup_version.size() ?
+  LogBabblePrintf (".ini setup_version is %s, our setup_version is %s", ini_setup_version.size() ?
        ini_setup_version.c_str () : "(null)",
        setup_version);
   if (ini_setup_version.size ())
diff --git a/msg.cc b/msg.cc
index d0cbfa2..b7d96bb 100644
--- a/msg.cc
+++ b/msg.cc
@@ -31,16 +31,6 @@ static const char *cvsid =
 #include "dialog.h"
 #include "state.h"
 
-void
-msg (const char *fmt, ...)
-{
-  char buf[2000];
-  va_list args;
-  va_start (args, fmt);
-  vsnprintf (buf, 2000, fmt, args);
-  OutputDebugString (buf);
-}
-
 int
 mbox (HWND owner, const char *buf, const char *name, int type)
 {
diff --git a/msg.h b/msg.h
index 72551c4..4810057 100644
--- a/msg.h
+++ b/msg.h
@@ -18,12 +18,6 @@
 
 #include "win32.h"
 
-/* This is for "printf"-like debugging.  Messages go to
-   OutputDebugString, which can be seen while debugging under GDB or
-   via a debug message monitor. */
-
-void msg (const char *fmt, ...);
-
 /* This pops up a dialog with text from the string table ("id"), which
    is interpreted like printf.  The program exits when the user
    presses OK. */
diff --git a/simpsock.cc b/simpsock.cc
index b1bb5f0..852f043 100644
--- a/simpsock.cc
+++ b/simpsock.cc
@@ -27,7 +27,7 @@ static const char *cvsid =
 #include <stdlib.h>
 
 #include "simpsock.h"
-#include "msg.h"
+#include "LogSingleton.h"
 
 #define SSBUFSZ 1024
 
@@ -61,7 +61,7 @@ SimpleSocket::SimpleSocket (const char *hostname, int port)
       he = gethostbyname (hostname);
       if (!he)
 	{
-	  msg ("Can't resolve `%s'\n", hostname);
+	  LogPlainPrintf ("Can't resolve `%s'\n", hostname);
 	  return;
 	}
       memcpy (ip, he->h_addr_list[0], 4);
@@ -70,7 +70,7 @@ SimpleSocket::SimpleSocket (const char *hostname, int port)
   s = socket (AF_INET, SOCK_STREAM, 0);
   if (s == INVALID_SOCKET)
     {
-      msg ("Can't create socket, %d", WSAGetLastError ());
+      LogPlainPrintf ("Can't create socket, %d", WSAGetLastError ());
       return;
     }
 
@@ -83,7 +83,7 @@ SimpleSocket::SimpleSocket (const char *hostname, int port)
 
   if (connect (s, (sockaddr *) & name, sizeof (name)))
     {
-      msg ("Can't connect to %s:%d", hostname, port);
+      LogPlainPrintf ("Can't connect to %s:%d", hostname, port);
       closesocket (s);
       s = INVALID_SOCKET;
       return;
-- 
2.1.4

[PATCH setup 3/4] Log errors from archive_tar.cc

[Jon TURNEY]

archive_tar.cc contains some output directly using fprintf(stderr,), convert
that to using the printf-style log adaptors.
---
 ChangeLog      |  5 +++++
 archive_tar.cc | 25 +++++++++----------------
 2 files changed, 14 insertions(+), 16 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 472030e..bc47a54 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,10 @@
 2015-03-04  Jon TURNEY  <jon.turney@dronecode.org.uk>
 
+	* archive_tar.cc (archive_tar): Convert from fprintf(stderr, ...)
+	to LogBabblePrintf() or LogPlainPrintf() as appropriate.
+
+2015-03-04  Jon TURNEY  <jon.turney@dronecode.org.uk>
+
 	* msg.cc (msg): Remove.
 	* msg.h (msg): Ditto.
 	* compress_xz.cc (read, bid_xz, bid_lzma): Convert from msg() to
diff --git a/archive_tar.cc b/archive_tar.cc
index 71dcb57..e81efeb 100644
--- a/archive_tar.cc
+++ b/archive_tar.cc
@@ -46,18 +46,11 @@ static int err;
 
 static char buf[512];
 
-int _tar_verbose = 0;
-FILE *_tar_vfile = 0;
-#define vp if (_tar_verbose) fprintf
-#define vp2 if (_tar_verbose>1) fprintf
-
 archive_tar::archive_tar (io_stream * original)
 {
   archive_children = 0;
-  if (_tar_vfile == 0)
-    _tar_vfile = stderr;
 
-  vp2 (_tar_vfile, "tar: open `%p'\n", original);
+  LogBabblePrintf("tar: open `%p'\n", original);
 
   if (!original)
     {
@@ -170,8 +163,8 @@ archive_tar::next_file_name ()
   sscanf (state.tar_header.size, "%Io", &state.file_length);
   state.file_offset = 0;
 
-//  vp2 (_tar_vfile, "%c %9d %s\n", state.tar_header.typeflag,
-//      state.file_length, state.filename);
+  LogBabblePrintf ("%c %9d %s\n", state.tar_header.typeflag,
+                   state.file_length, state.filename);
 
   switch (state.tar_header.typeflag)
     {
@@ -182,8 +175,8 @@ archive_tar::next_file_name ()
       if (state.file_length > CYG_PATH_MAX)
 	{
 	  skip_file ();
-	  fprintf (stderr, "error: long file name exceeds %d characters\n",
-		   CYG_PATH_MAX);
+	  LogPlainPrintf( "error: long file name exceeds %d characters\n",
+                          CYG_PATH_MAX);
 	  err++;
 	  state.parent->read (&state.tar_header, 512);
 	  sscanf (state.tar_header.size, "%Io", &state.file_length);
@@ -215,8 +208,8 @@ archive_tar::next_file_name ()
     case '3':			/* char */
     case '4':			/* block */
     case '6':			/* fifo */
-      fprintf (stderr, "warning: not extracting special file %s\n",
-	       state.filename);
+      LogPlainPrintf ("warning: not extracting special file %s\n",
+                      state.filename);
       err++;
       return next_file_name ();
 
@@ -233,8 +226,8 @@ archive_tar::next_file_name ()
       return state.filename;
 
     default:
-      fprintf (stderr, "error: unknown (or unsupported) file type `%c'\n",
-	       state.tar_header.typeflag);
+      LogPlainPrintf ("error: unknown (or unsupported) file type `%c'\n",
+                      state.tar_header.typeflag);
       err++;
       skip_file ();
       return next_file_name ();
-- 
2.1.4

[PATCH setup 4/4] Silently ignore 'x' and 'g' type tar extended headers

[Jon TURNEY]

Silently ignore 'g' and 'x' type tar extended headers, rather than warning about
"unknown (or unsupported) file type 'x'".

It seems that base-files has an 'x' extended header for each file, apparently to
store the mtime.
---
 ChangeLog      | 5 +++++
 archive_tar.cc | 3 +++
 2 files changed, 8 insertions(+)

diff --git a/ChangeLog b/ChangeLog
index bc47a54..75530de 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,10 @@
 2015-03-04  Jon TURNEY  <jon.turney@dronecode.org.uk>
 
+	* archive_tar.cc (next_file_name): Silence error for 'g' and 'x'
+	type headers.
+
+2015-03-04  Jon TURNEY  <jon.turney@dronecode.org.uk>
+
 	* archive_tar.cc (archive_tar): Convert from fprintf(stderr, ...)
 	to LogBabblePrintf() or LogPlainPrintf() as appropriate.
 
diff --git a/archive_tar.cc b/archive_tar.cc
index e81efeb..131591d 100644
--- a/archive_tar.cc
+++ b/archive_tar.cc
@@ -229,6 +229,9 @@ archive_tar::next_file_name ()
       LogPlainPrintf ("error: unknown (or unsupported) file type `%c'\n",
                       state.tar_header.typeflag);
       err++;
+      /* fall through */
+    case 'g':			/* POSIX.1-2001 global extended header */
+    case 'x':			/* POSIX.1-2001 extended header */
       skip_file ();
       return next_file_name ();
     }
-- 
2.1.4

Re: [PATCH setup 3/4] Log errors from archive_tar.cc

[Corinna Vinschen]

[-- Attachment #1: Type: text/plain, Size: 640 bytes --]

On Mar  5 13:44, Jon TURNEY wrote:
> @@ -170,8 +163,8 @@ archive_tar::next_file_name ()
>    sscanf (state.tar_header.size, "%Io", &state.file_length);
>    state.file_offset = 0;
>  
> -//  vp2 (_tar_vfile, "%c %9d %s\n", state.tar_header.typeflag,
> -//      state.file_length, state.filename);
> +  LogBabblePrintf ("%c %9d %s\n", state.tar_header.typeflag,
> +                   state.file_length, state.filename);

Is it really worth to add this to the babble?


Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Maintainer                 cygwin AT cygwin DOT com
Red Hat

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH setup 0/4] Setup logging fixes

[Corinna Vinschen]

[-- Attachment #1: Type: text/plain, Size: 1119 bytes --]

On Mar  5 13:44, Jon TURNEY wrote:
> On 02/03/2015 16:32, Corinna Vinschen wrote:
> > On Mar  2 13:55, Jon TURNEY wrote:
> >>
> >> Future work: there are some other uses of msg() to report real errors.  At the
> >> moment, these are completely invisible unless you are running setup under a
> >> debugger.  These should be converted to use the logger.
> [...]
> > Approved, under the assumption that you'll rework the other msg() calls
> > as well ;)
> 
> :)
> 
> Also change some of the uses of fprintf(stderr, ...) to report errors, which doesn't get logged.
> 
> This then exposes a bit of log spam in archive_tar.cc, which is silenced.
> 
> Jon TURNEY (4):
>   Add Log adaptors for printf-style output
>   Remove msg() and convert to log output
>   Log errors from archive_tar.cc
>   Silently ignore 'x' and 'g' type tar extended headers

Barring the single question I'm asking in other mail, these patches
look good.


Thanks,
Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Maintainer                 cygwin AT cygwin DOT com
Red Hat

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH setup 3/4] Log errors from archive_tar.cc

[Jon TURNEY]

On 05/03/2015 14:17, Corinna Vinschen wrote:
> On Mar  5 13:44, Jon TURNEY wrote:
>> @@ -170,8 +163,8 @@ archive_tar::next_file_name ()
>>     sscanf (state.tar_header.size, "%Io", &state.file_length);
>>     state.file_offset = 0;
>>
>> -//  vp2 (_tar_vfile, "%c %9d %s\n", state.tar_header.typeflag,
>> -//      state.file_length, state.filename);
>> +  LogBabblePrintf ("%c %9d %s\n", state.tar_header.typeflag,
>> +                   state.file_length, state.filename);
>
> Is it really worth to add this to the babble?

Erm no.  This should stay under 'if (_tar_verbose)' or something.

Re: [PATCH setup 3/4] Log errors from archive_tar.cc

[Corinna Vinschen]

[-- Attachment #1: Type: text/plain, Size: 869 bytes --]

On Mar  5 15:09, Jon TURNEY wrote:
> On 05/03/2015 14:17, Corinna Vinschen wrote:
> >On Mar  5 13:44, Jon TURNEY wrote:
> >>@@ -170,8 +163,8 @@ archive_tar::next_file_name ()
> >>    sscanf (state.tar_header.size, "%Io", &state.file_length);
> >>    state.file_offset = 0;
> >>
> >>-//  vp2 (_tar_vfile, "%c %9d %s\n", state.tar_header.typeflag,
> >>-//      state.file_length, state.filename);
> >>+  LogBabblePrintf ("%c %9d %s\n", state.tar_header.typeflag,
> >>+                   state.file_length, state.filename);
> >
> >Is it really worth to add this to the babble?
> 
> Erm no.  This should stay under 'if (_tar_verbose)' or something.

Yeah.  With this change, push it.


Thanks,
Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Maintainer                 cygwin AT cygwin DOT com
Red Hat

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH setup 4/4] Silently ignore 'x' and 'g' type tar extended headers

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 814 bytes --]

On 03/05/2015 06:44 AM, Jon TURNEY wrote:
> Silently ignore 'g' and 'x' type tar extended headers, rather than warning about
> "unknown (or unsupported) file type 'x'".
> 
> It seems that base-files has an 'x' extended header for each file, apparently to
> store the mtime.

Would we ever need to actually honor extended attributes to correctly
unpack a particular package?  I guess as long as we don't have things
like xattr for fine-grained capabilities as an alternative to
traditional setuid, we haven't needed attributes so far.  At any rate,
ignoring extensions until we have both a need and the code for honoring
the extension makes sense, so my question shouldn't stall your patch.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [PATCH setup 4/4] Silently ignore 'x' and 'g' type tar extended headers

[Achim Gratz]

Jon TURNEY writes:
> It seems that base-files has an 'x' extended header for each file, apparently to
> store the mtime.

That's a result of me having built that file on openSUSE and openSUSE's
decision to default to POSIX format instead of GNU.


Regards,
Achim.
-- 
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

Factory and User Sound Singles for Waldorf Q+, Q and microQ:
http://Synth.Stromeko.net/Downloads.html#WaldorfSounds

Re: [PATCH setup 4/4] Silently ignore 'x' and 'g' type tar extended headers

[Jon TURNEY]

On 06/03/2015 06:08, Achim Gratz wrote:
> Jon TURNEY writes:
>> It seems that base-files has an 'x' extended header for each file, apparently to
>> store the mtime.
>
> That's a result of me having built that file on openSUSE and openSUSE's
> decision to default to POSIX format instead of GNU.

Hmm... maybe I should drop this patch, if the correct thing to do is to 
build base-files with tar --format=gnu?

Re: [PATCH setup 4/4] Silently ignore 'x' and 'g' type tar extended headers

[Corinna Vinschen]

[-- Attachment #1: Type: text/plain, Size: 761 bytes --]

On Mar 10 14:01, Jon TURNEY wrote:
> On 06/03/2015 06:08, Achim Gratz wrote:
> >Jon TURNEY writes:
> >>It seems that base-files has an 'x' extended header for each file, apparently to
> >>store the mtime.
> >
> >That's a result of me having built that file on openSUSE and openSUSE's
> >decision to default to POSIX format instead of GNU.
> 
> Hmm... maybe I should drop this patch, if the correct thing to do is to
> build base-files with tar --format=gnu?

If your patch allows to unpack a POSIX tar archive correctly, without
generating a warning... why would you like to drop it?


Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Maintainer                 cygwin AT cygwin DOT com
Red Hat

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH setup 4/4] Silently ignore 'x' and 'g' type tar extended headers

[Jon TURNEY]

On 10/03/2015 14:23, Corinna Vinschen wrote:
> On Mar 10 14:01, Jon TURNEY wrote:
>> On 06/03/2015 06:08, Achim Gratz wrote:
>>> Jon TURNEY writes:
>>>> It seems that base-files has an 'x' extended header for each file, apparently to
>>>> store the mtime.
>>>
>>> That's a result of me having built that file on openSUSE and openSUSE's
>>> decision to default to POSIX format instead of GNU.
>>
>> Hmm... maybe I should drop this patch, if the correct thing to do is to
>> build base-files with tar --format=gnu?
>
> If your patch allows to unpack a POSIX tar archive correctly, without
> generating a warning... why would you like to drop it?

POSIX tar archive are (potentially) not unpacked strictly correctly in 
both cases, as they contain extra information that setup does not interpret.

All this patch changes is if we warn about that loss of information, or 
silently drop it.

Re: [PATCH setup 4/4] Silently ignore 'x' and 'g' type tar extended headers

[Corinna Vinschen]

[-- Attachment #1: Type: text/plain, Size: 1325 bytes --]

On Mar 10 15:23, Jon TURNEY wrote:
> On 10/03/2015 14:23, Corinna Vinschen wrote:
> >On Mar 10 14:01, Jon TURNEY wrote:
> >>On 06/03/2015 06:08, Achim Gratz wrote:
> >>>Jon TURNEY writes:
> >>>>It seems that base-files has an 'x' extended header for each file, apparently to
> >>>>store the mtime.
> >>>
> >>>That's a result of me having built that file on openSUSE and openSUSE's
> >>>decision to default to POSIX format instead of GNU.
> >>
> >>Hmm... maybe I should drop this patch, if the correct thing to do is to
> >>build base-files with tar --format=gnu?
> >
> >If your patch allows to unpack a POSIX tar archive correctly, without
> >generating a warning... why would you like to drop it?
> 
> POSIX tar archive are (potentially) not unpacked strictly correctly in both
> cases, as they contain extra information that setup does not interpret.
> 
> All this patch changes is if we warn about that loss of information, or
> silently drop it.

I thought the difference is just in the header.  Also, is the extra
information required for our use case?  Anyway, if you're not
comfortable with the change, by all means back it out.


Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Maintainer                 cygwin AT cygwin DOT com
Red Hat

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH setup 4/4] Silently ignore 'x' and 'g' type tar extended headers

[Achim Gratz]

Jon TURNEY writes:
>> That's a result of me having built that file on openSUSE and openSUSE's
>> decision to default to POSIX format instead of GNU.
>
> Hmm... maybe I should drop this patch, if the correct thing to do is
> to build base-files with tar --format=gnu?

GNU's been threatening to make POSIX the default for over a decade now,
it's just that some openSUSE maintainer sprung into action a bit
prematurely on that issue.  The PAX extended headers are not needed by
setup whatever their content may be AFAIK, but it would be nice if they
didn't produce spuroius warnings or errors.


Regards,
Achim.
-- 
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

Waldorf MIDI Implementation & additional documentation:
http://Synth.Stromeko.net/Downloads.html#WaldorfDocs