From: john at keeping.me.uk (John Keeping)
Subject: [PATCH] Use strbuf for reading configuration files
Date: Tue, 4 Jun 2013 19:23:03 +0100 [thread overview]
Message-ID: <20130604182303.GO1072@serenity.lan> (raw)
In-Reply-To: <1370357273-4532-1-git-send-email-cgit@cryptocrack.de>
On Tue, Jun 04, 2013 at 04:47:53PM +0200, Lukas Fleischer wrote:
> Use struct strbuf from Git instead of fixed-size buffers to remove the
> limit on the length of configuration file lines and refactor
> read_config_line() to improve readability.
>
> Note that this also fixes a buffer overflow that existed with the
> original fixed-size buffer implementation.
>
> Signed-off-by: Lukas Fleischer <cgit at cryptocrack.de>
> ---
Is there any reason not to use strbuf_getline and
string_list_split_in_place if we're using strbuf here?
> configfile.c | 65 ++++++++++++++++++++++++++++++------------------------------
> configfile.h | 2 ++
> 2 files changed, 35 insertions(+), 32 deletions(-)
>
> diff --git a/configfile.c b/configfile.c
> index d98989c..e6ad1d6 100644
> --- a/configfile.c
> +++ b/configfile.c
> @@ -31,45 +31,44 @@ static void skip_line(FILE *f)
> ;
> }
>
> -static int read_config_line(FILE *f, char *line, const char **value, int bufsize)
> +static int read_config_line(FILE *f, struct strbuf *name, struct strbuf *value)
> {
> - int i = 0, isname = 0;
> + int c = next_char(f);
>
> - *value = NULL;
> - while (i < bufsize - 1) {
> - int c = next_char(f);
> - if (!isname && (c == '#' || c == ';')) {
> - skip_line(f);
> - continue;
> - }
> - if (!isname && isspace(c))
> - continue;
> + strbuf_reset(name);
> + strbuf_reset(value);
>
> - if (c == '=' && !*value) {
> - line[i] = 0;
> - *value = &line[i + 1];
> - } else if (c == '\n' && !isname) {
> - i = 0;
> - continue;
> - } else if (c == '\n' || c == EOF) {
> - line[i] = 0;
> - break;
> - } else {
> - line[i] = c;
> - }
> - isname = 1;
> - i++;
> + /* Skip comments and preceding spaces. */
> + while (c == '#' || c == ';') {
> + skip_line(f);
> + c = next_char(f);
> + }
> + while (isspace(c))
> + c = next_char(f);
> +
> + /* Read variable name. */
> + while (c != '=') {
> + if (c == '\n' || c == EOF)
> + return 0;
> + strbuf_addch(name, c);
> + c = next_char(f);
> }
> - line[i + 1] = 0;
> - return i;
> +
> + /* Read variable value. */
> + c = next_char(f);
> + while (c != '\n' && c != EOF) {
> + strbuf_addch(value, c);
> + c = next_char(f);
> + }
> +
> + return 1;
> }
>
> int parse_configfile(const char *filename, configfile_value_fn fn)
> {
> static int nesting;
> - int len;
> - char line[256];
> - const char *value;
> + struct strbuf name = STRBUF_INIT;
> + struct strbuf value = STRBUF_INIT;
> FILE *f;
>
> /* cancel deeply nested include-commands */
> @@ -78,10 +77,12 @@ int parse_configfile(const char *filename, configfile_value_fn fn)
> if (!(f = fopen(filename, "r")))
> return -1;
> nesting++;
> - while ((len = read_config_line(f, line, &value, sizeof(line))) > 0)
> - fn(line, value);
> + while (read_config_line(f, &name, &value))
> + fn(name.buf, value.buf);
> nesting--;
> fclose(f);
> + strbuf_release(&name);
> + strbuf_release(&value);
> return 0;
> }
>
> diff --git a/configfile.h b/configfile.h
> index 04235e5..af7ca19 100644
> --- a/configfile.h
> +++ b/configfile.h
> @@ -1,6 +1,8 @@
> #ifndef CONFIGFILE_H
> #define CONFIGFILE_H
>
> +#include "cgit.h"
> +
> typedef void (*configfile_value_fn)(const char *name, const char *value);
>
> extern int parse_configfile(const char *filename, configfile_value_fn fn);
> --
> 1.8.3.450.gf3f2a46
next prev parent reply other threads:[~2013-06-04 18:23 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-06-03 20:21 configfile.c:63:14: warning: array subscript is above array [-Warray-bounds] koverstreet
2013-06-04 10:34 ` Jason
2013-06-04 13:57 ` cgit
2013-06-04 14:47 ` [PATCH] Use strbuf for reading configuration files cgit
2013-06-04 15:43 ` cgit
2013-06-04 17:29 ` Jason
2013-06-04 17:35 ` Jason
2013-06-04 17:39 ` cgit
2013-06-04 17:40 ` Jason
2013-06-04 17:49 ` Jason
2013-06-04 18:23 ` john [this message]
2013-06-05 11:20 ` cgit
2013-06-05 11:17 ` [PATCH v2] " cgit
2013-06-13 21:25 ` cgit
2013-08-02 17:13 ` cgit
2013-08-02 18:14 ` Jason
2013-08-12 18:52 ` Jason
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20130604182303.GO1072@serenity.lan \
--to=cgit@lists.zx2c4.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).