Re: [Bug-wget] Writing to a Read-Only directory

[Tim Rühsen]

Am Mittwoch, 10. Februar 2016, 18:58:35 schrieb Gisle Vanem:
> Tim Ruehsen wrote:
> > I fixed that issue in a tiny commit that I just pushed.
> > 
> > BTW, I remember we had this or a similar issue before... though I couldn't
> > find it with a quick search.
> 
> Sorry Tim, the error-message is still the same.
> There are several return-paths in logprintf() where 'errno_saved'
> isn't restored. This is what I did to prevent losing 'errno':

Hmmm, sorry if I was a bit too fast. The issue was fixed for me here:

$ mkdir xxx
$ chmod 500 xxx
$ cd xxx
$ $ LC_ALL=C ../src/wget http://www.watt-32.net/misc/ASMBSW.dll
--2016-02-10 21:03:28--  http://www.watt-32.net/misc/ASMBSW.dll
Resolving www.watt-32.net (www.watt-32.net)... 46.30.212.248
Connecting to www.watt-32.net (www.watt-32.net)|46.30.212.248|:80... 
connected.
HTTP request sent, awaiting response... 200 OK
Length: 107008 (104K) [application/x-msdos-program]
ASMBSW.dll: Permission denied

Cannot write to 'ASMBSW.dll' (Permission denied).


But of course, if you assume anything going wrong in check_redirect_output() 
then we have to restore errno directly after it returns. 'goto quit' is IMHO 
not needed. CHECK_VERBOSE() doesn't change errno.

That is IMO a bit less code changes, WDYT:


diff --git a/src/log.c b/src/log.c
index d4beda1..a1338ca 100644
--- a/src/log.c
+++ b/src/log.c
@@ -351,6 +351,7 @@ logputs (enum log_options o, const char *s)
 {
   FILE *fp;
   FILE *warcfp;
+  int errno_save = errno;
 
   check_redirect_output ();
   if (o == LOG_PROGRESS)
@@ -358,10 +359,14 @@ logputs (enum log_options o, const char *s)
   else
     fp = get_log_fp ();
 
+  errno = errno_save;
+
   if (fp == NULL)
     return;
 
   warcfp = get_warc_log_fp ();
+  errno = errno_save;
+
   CHECK_VERBOSE (o);
 
   FPUTS (s, fp);
@@ -373,6 +378,8 @@ logputs (enum log_options o, const char *s)
     logflush ();
   else
     needs_flushing = true;
+
+  errno = errno_save;
 }
 
 struct logvprintf_state {
@@ -546,6 +553,7 @@ logprintf (enum log_options o, const char *fmt, ...)
   int errno_saved = errno;
 
   check_redirect_output ();
+  errno = errno_saved;
   if (inhibit_logging)
     return;
   CHECK_VERBOSE (o);





> --- a/log.c 2016-02-10 18:09:07
> +++ b/log.c 2016-02-10 18:53:25
> @@ -277,21 +277,21 @@
>      {                                           \
>      case LOG_PROGRESS:                          \
>        if (!opt.show_progress)                   \
> -        return;                                 \
> +        goto quit;                              \
>        break;                                    \
>      case LOG_ALWAYS:                            \
>        break;                                    \
>      case LOG_NOTQUIET:                          \
>        if (opt.quiet)                            \
> -        return;                                 \
> +        goto quit;                              \
>        break;                                    \
>      case LOG_NONVERBOSE:                        \
>        if (opt.verbose || opt.quiet)             \
> -        return;                                 \
> +        goto quit;                              \
>        break;                                    \
>      case LOG_VERBOSE:                           \
>        if (!opt.verbose)                         \
> -        return;                                 \
> +        goto quit;                              \
>      }
> 
>  /* Returns the file descriptor for logging.  This is LOGFP, except if
> @@ -351,6 +351,7 @@
>  {
>    FILE *fp;
>    FILE *warcfp;
> +  int errno_saved = errno;
> 
>    check_redirect_output ();
>    if (o == LOG_PROGRESS)
> @@ -359,7 +360,7 @@
>      fp = get_log_fp ();
> 
>    if (fp == NULL)
> -    return;
> +    goto quit;
> 
>    warcfp = get_warc_log_fp ();
>    CHECK_VERBOSE (o);
> @@ -373,6 +374,9 @@
>      logflush ();
>    else
>      needs_flushing = true;
> +
> +quit:
> +  errno = errno_saved;
>  }
> 
>  struct logvprintf_state {
> @@ -547,7 +551,8 @@
> 
>    check_redirect_output ();
>    if (inhibit_logging)
> -    return;
> +    goto quit;
> +
>    CHECK_VERBOSE (o);
> 
>    xzero (lpstate);
> @@ -563,6 +568,7 @@
>      }
>    while (!done);
> 
> +quit:
>    errno = errno_saved;
>  }
> 
> ------------------------
> 
> The question is if errno caused by logprintf() gets lost and
> caused havoc elsewhere!?

signature.asc
Description: This is a digitally signed message part.