[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Bug-wget] [PATCH] replaced read_whole_file() by getline()
From: |
Giuseppe Scrivano |
Subject: |
Re: [Bug-wget] [PATCH] replaced read_whole_file() by getline() |
Date: |
Mon, 13 May 2013 23:24:38 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.2 (gnu/linux) |
thanks for the patch. I have few comments:
Tim Rühsen <address@hidden> writes:
> diff --git a/src/ChangeLog b/src/ChangeLog
> index 84a9645..3b6733e 100644
> --- a/src/ChangeLog
> +++ b/src/ChangeLog
> @@ -1,5 +1,10 @@
> 2013-05-09 Tim Ruehsen <address@hidden>
>
> + * cookies.c, ftp-ls.c, ftp.c, init.c, netrc.c, utils.c, utils.h:
> + replaced read_whole_file() by getline().
> +
> +2013-05-09 Tim Ruehsen <address@hidden>
> +
please fix it specifying the functions too, as done in the ChangeLog file.
> diff --git a/src/ftp-ls.c b/src/ftp-ls.c
> index 3056651..401ae77 100644
> --- a/src/ftp-ls.c
> +++ b/src/ftp-ls.c
> @@ -68,16 +68,17 @@ symperms (const char *s)
> replaces all <TAB> character with <SPACE>. Returns the length of the
> modified line. */
> static int
> -clean_line(char *line)
> +clean_line(char *line, int len)
since you are touching it, please add a space before '('
> + while ((len = getline (&line, &bufsize, fp)) > 0)
> {
> - len = clean_line (line);
> + len = clean_line (line, len);
> /* Skip if total... */
> if (!strncasecmp (line, "total", 5))
> - {
> - xfree (line);
> continue;
> - }
please fix the indentation for the "continue" statement.
> /* Get the first token (permissions). */
> tok = strtok (line, " ");
> if (!tok)
> - {
> - xfree (line);
> continue;
> - }
>
same here.
> + i = clean_line (line, i);
> + if (i <= 0)
> + continue; /* Ignore blank line. */
and here.
> +
> + if ((j == 0) && (line[i - 1] == ']'))
> + {
> + /* Found Directory heading line. Next non-blank line
> + is significant. */
> + j = 1;
> + }
> + else if (!strncmp (line, "Total of ", 9))
> + {
> + /* Found "Total of ..." footing line. No valid data
> + will follow (empty directory). */
> + i = 0; /* Arrange for early exit. */
> + break;
> + }
> + else
> + {
> + break; /* Must be significant data. */
> + }
> }
please not use tabs to indent here.
> - if (!line)
> + i = getline (&line, &bufsize, fp);
and here.
> + if (i <= 0)
> {
> DEBUGP (("EOF. Leaving listing parser.\n"));
> break;
> @@ -853,14 +828,14 @@ ftp_parse_vms_ls (const char *file)
> /* Second line must begin with " ". Otherwise, it's a first
> line (and we may be confused).
> */
> + i = clean_line (line, i);
here too.
> - for (i = 0; i < strlen( tok); i++)
> + for (i = 0; i < strlen(tok); i++)
thanks :-)
> - if (line != NULL)
> + i = getline (&line, &bufsize, fp);
tab.
> diff --git a/src/ftp.c b/src/ftp.c
> index b585631..fbfbe8f 100644
> --- a/src/ftp.c
> +++ b/src/ftp.c
> @@ -1367,18 +1367,20 @@ Error in server response, closing control
> connection.\n"));
> logprintf (LOG_ALWAYS, "%s: %s\n", con->target, strerror (errno));
> else
> {
> - char *line;
> - /* The lines are being read with read_whole_line because of
> + char *line = NULL;
> + size_t bufsize = 0;
> + ssize_t len;
it doesn't seem correctly indented.
Except these minor comments, it seems correct. Thanks again for your
contribution.
--
Giuseppe