Re: [Bug-wget] [PATCH] Keep fetched URLs in POSIX extended attributes

[Ander Juaristi]

Hi Sean,

Great patch!

But I just had some unimportant comments about coding style ;D

On 21/07/16 06:33, Sean Burford wrote:
> diff --git a/src/ftp.c b/src/ftp.c
> index 88a9777..27d90d6 100644
> --- a/src/ftp.c
> +++ b/src/ftp.c
> @@ -52,6 +52,9 @@ as that of the covered work.  */
>  #include "recur.h"              /* for INFINITE_RECURSION */
>  #include "warc.h"
>  #include "c-strcase.h"
> +#ifdef ENABLE_XATTR
> +#include "xattr.h"
> +#endif
>  
>  #ifdef __VMS
>  # include "vms.h"
> @@ -1546,6 +1549,13 @@ Error in server response, closing control 
> connection.\n"));
>    tmrate = retr_rate (rd_size, con->dltime);
>    total_download_time += con->dltime;
>  
> +#ifdef ENABLE_XATTR
> +  if (opt.enable_xattr)
> +    {
> +      set_file_metadata (u->url, NULL, fp);
> +    }
> +#endif
> +

remove the braces, and indent the if body by two spaces

>    fd_close (local_sock);
>    /* Close the local file.  */
>    if (!output_stream || con->cmd & DO_LIST)
> diff --git a/src/http.c b/src/http.c
> index 7e60a07..0cd142c 100644
> --- a/src/http.c
> +++ b/src/http.c
> @@ -66,6 +66,9 @@ as that of the covered work.  */
>  # include "metalink.h"
>  # include "xstrndup.h"
>  #endif
> +#ifdef ENABLE_XATTR
> +#include "xattr.h"
> +#endif
>  
>  #ifdef TESTING
>  #include "test.h"
> @@ -2892,8 +2895,8 @@ fail:
>     If PROXY is non-NULL, the connection will be made to the proxy
>     server, and u->url will be requested.  */
>  static uerr_t
> -gethttp (struct url *u, struct http_stat *hs, int *dt, struct url *proxy,
> -         struct iri *iri, int count)
> +gethttp (struct url *u, struct url *original_url, struct http_stat *hs,
> +         int *dt, struct url *proxy, struct iri *iri, int count)
>  {
>    struct request *req = NULL;
>  
> @@ -3754,6 +3757,20 @@ gethttp (struct url *u, struct http_stat *hs, int *dt, 
> struct url *proxy,
>        goto cleanup;
>      }
>  
> +#ifdef ENABLE_XATTR
> +  if (opt.enable_xattr)
> +    {
> +      if (original_url != u)
> +        {
> +          set_file_metadata (u->url, original_url->url, fp);
> +        }
> +      else
> +        {
> +          set_file_metadata (u->url, NULL, fp);
> +        }
> +    }
> +#endif
> +

likewise, for the inner if-else

>    err = read_response_body (hs, sock, fp, contlen, contrange,
>                              chunked_transfer_encoding,
>                              u->url, warc_timestamp_str,
> @@ -3972,7 +3989,7 @@ http_loop (struct url *u, struct url *original_url, 
> char **newloc,
>          *dt &= ~SEND_NOCACHE;
>  
>        /* Try fetching the document, or at least its head.  */
> -      err = gethttp (u, &hstat, dt, proxy, iri, count);
> +      err = gethttp (u, original_url, &hstat, dt, proxy, iri, count);
>  
>        /* Time?  */
>        tms = datetime_str (time (NULL));

And here,

> diff --git a/src/xattr.c b/src/xattr.c
> new file mode 100644
> index 0000000..360b032
> --- /dev/null
> +++ b/src/xattr.c
> @@ -0,0 +1,71 @@
> +/* xattr.h -- POSIX Extended Attribute support.
> +
> +   Copyright (C) 2016 Free Software Foundation, Inc.
> +
> +   This program is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; either version 3, or (at your option)
> +   any later version.
> +
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with this program; if not, see <http://www.gnu.org/licenses/>.  */
> +
> +#include "wget.h"
> +
> +#include <stdio.h>
> +#include <string.h>
> +
> +#include "log.h"
> +#include "xattr.h"
> +
> +#ifdef USE_XATTR
> +
> +static int
> +write_xattr_metadata (const char *name, const char *value, FILE *fp) {
> +  int retval = -1;
> +  if (name && value && fp)
> +    {
> +      retval = fsetxattr (fileno(fp), name, value, strlen(value), 0);

we use spaces before the parentheses:
fileno (fp) and strlen (value)

> +      /* FreeBSD's extattr_set_fd returns the length of the extended 
> attribute. */
> +      retval = (retval < 0)? retval : 0;

and before the '?'

> +    }
> +  return retval;
> +}
> +
> +#else /* USE_XATTR */
> +

signature.asc
Description: OpenPGP digital signature