Re: [Bug-wget] Conditional GET requests

[Tim Ruehsen]

Hi Hubert,

On Monday 18 May 2015 23:57:57 Hubert Tarasiuk wrote:
> W dniu 18.05.2015 o 21:34, Tim Rühsen pisze:
> And by the way do you know how to silence
> the variable-sized array GCC warnings without silencing other ISO
> warnings (like the one above)?

-Wno-vla

But don't switch it off for C89 projects like Wget. VLA is a C99 feature that 
we try to avoid in Wget.

I personally use gcc 4.9.2 with
-std=c89 -pedantic -O2 -g -Wall -Wextra -Wstrict-prototypes -Wold-style-
definition -Wwrite-strings -Wshadow -Wformat -Wformat-security -Wunreachable-
code -Wstrict-prototypes -Wmissing-prototypes -Wold-style-definition -
Wlogical-op -Wsuggest-attribute=noreturn -Wsuggest-attribute=format -
D_FORTIFY_SOURCE=2 -D _GNU_SOURCE

Uups, I should use -Og for edit/compile/debug ;-)

clang has -Weverything, but you have to limit the output by using a few -
Wno-... flags. And clang compiles much slower than gcc, at least with the 
above mentioned flags.

> I am having some doubts about this part of code:
> >   /* Handle the case when server ignores If-Modified-Since header.  */
> >   if (cond_get && statcode == HTTP_STATUS_OK && hs->remote_time)
> >   
> >     {
> >     
> >       time_t tmr;
> >       tmr = http_atotm (hs->remote_time);
> >       if (tmr != (time_t) -1 && tmr <= hs->orig_file_tstamp
> >       
> >           && (contlen == -1 || contlen == hs->orig_file_size))
> >         
> >         {
> >         
> >           logprintf (LOG_VERBOSE, _("Server ignored If-Modified-Since
> >           header "
> >           
> >                                     "for file %s. "
> >                                     "You might want to add
> >                                     --no-if-modified-since "
> >                                     "option.\n\n"),
> >                                     quote(hs->local_file));
> >           
> >           *dt |= RETROKF;
> >           CLOSE_INVALIDATE (sock);
> >           retval = RETRUNNEEDED;
> >           goto cleanup;
> >         
> >         }
> >     
> >     }
> 
> Do you think that it is ok? Specifically in warc mode. It is kind of
> fall-back to old-style timestamping (there would be no body at all in
> response to head request), but I am wondering if it is ok to discard
> response body in this case (when warc mode is enabled). Or perhaps
> should we store it as usual?

I am not aware of all WARC use cases... so, I just can't say.
Despite WARC, this behavior seems straight forward.

You could aggregate the code a bit, something like
if (cond_get) {
  if (HTTP_STATUS_NOT_MODIFIED) {
  } else if (statcode == HTTP_STATUS_OK && hs->remote_time) {
  }
}

Code is a bit more readable if you have a blank line between variable 
declarations and code.
And it is absolutely ok to have variable declaration and initialization in one 
line instead of two consecutive lines. Like in

      time_t tmr = http_atotm (hs->remote_time);
<blank line>
      if (...

Regards, Tim

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