Re: [Bug-wget] Segmentation fault with current development version of wget

[Giuseppe Scrivano]

thanks for your contribution, some comments inline:

Darshit Shah dar...@gmail.com writes:

 -  if (statcode == HTTP_STATUS_TEMPORARY_REDIRECT)
 -return NEWLOCATION_KEEP_POST;
 +  switch (statcode)
 +  {
 +case HTTP_STATUS_TEMPORARY_REDIRECT:
 +  return NEWLOCATION_KEEP_POST;
 +  break;
 +case HTTP_STATUS_MOVED_PERMANENTLY:
 +  if(opt.method  strcasecmp(opt.method,post)!=0)
 +return NEWLOCATION_KEEP_POST;
 +  break;
 +case HTTP_STATUS_MOVED_TEMPORARILY:
 +  if(opt.method  strcasecmp(opt.method,post)!=0)
 +return NEWLOCATION_KEEP_POST;
 +  break;

please follow the GNU Coding standards here.  Empty space between
function name and '(' and also leave spaces around the operator !=.

 +  if (opt.method  strcasecmp(opt.method,HEAD)==0)
 +setoptval(spider,1,spider);

same here.  Also leave a space after the commas.

I have seen these errors in other places too, please double check the
patch.

Could you please also add some comments in the documentation?

We can skip tests for now, since you are going to rewrite the unit tests
for your Summer of Code, but we will probably want something to stress
these features too.

-- 
Giuseppe

Re: [Bug-wget] [PATCH] MinGW compatibility fixes

[Giuseppe Scrivano]

Giuseppe Scrivano gscriv...@gnu.org writes:

 In my understanding, all new general patches should go into 'master', those 
 regarding metalink/multithreading should go into (experimental) 'parallel-
 wget' for later merging with 'master'.

 obviously it has to be into master too, thanks to have reported it.  I
 am going to cherry-pick from parallel-wget into master.

I have checked it, but I don't think that master needs this fix (the
patch includes also an improvement, but nothing blocking to be on master
ASAP).  Can you confirm this?

-- 
Giuseppe

Re: [Bug-wget] [PATCH] MinGW compatibility fixes

[Tim Rühsen]

Am Sonntag, 16. Juni 2013 schrieb Giuseppe Scrivano:
 Giuseppe Scrivano gscriv...@gnu.org writes:
 
  In my understanding, all new general patches should go into 'master', 
those 
  regarding metalink/multithreading should go into (experimental) 
'parallel-
  wget' for later merging with 'master'.
 
  obviously it has to be into master too, thanks to have reported it.  I
  am going to cherry-pick from parallel-wget into master.
 
 I have checked it, but I don't think that master needs this fix (the
 patch includes also an improvement, but nothing blocking to be on master
 ASAP).  Can you confirm this?

Some people stumbled upon the _PC_NAME_MAX stuff when compiling for Windows.
Just search the last 2 months for '_PC_NAME_MAX'.

I wondered why  Bykov Aleksey recommends ' Replace _PC_NAME_MAX with 256 
in  url.c:1620. Then run ...' (12th June). Since Rays patch should handle 
that. I'm shure I also send (at least one) patch regarding this issue, but I 
think it/they got lost or something.

However, it boils up and up again, but it is not a blocker that needs to be 
fixed ASAP. It's just annoying.

Regards, Tim

Re: [Bug-wget] Segmentation fault with current development version of wget

[Giuseppe Scrivano]

Thanks, the code seems correct but since you are going to write a lot of
code during your Summer of Code project, I will be very pedantic.  This
time I have just amended the changes before push your patch.


Darshit Shah dar...@gmail.com writes:

  Please be aware that Wget needs to know the size of the POST data in
 -advance.  Therefore the argument to @code{--post-file} must be a regular

we put two empty spaces after a sentence.  I have fixed this change in
other places too.


 +  switch (statcode)
 +  {

I have indented the '{' correctly.  Two empty spaces.  Same for the
closing curly bracket.


 +  if ((!(*dt  RETROKF)  !opt.content_on_error) || head_only || 
 (opt.method  strcasecmp (opt.method, GET) != 0))

long line, I have splitted it.


-- 
Giuseppe