* configfile.c:63:14: warning: array subscript is above array [-Warray-bounds]
@ 2013-06-03 20:21 koverstreet
2013-06-04 10:34 ` Jason
0 siblings, 1 reply; 17+ messages in thread
From: koverstreet @ 2013-06-03 20:21 UTC (permalink / raw)
Just saw that when building 52c926c with gcc 4.6.2, the line of code is
line[i + 1] = 0;
return i;
Took a look at the code and it looks like a real buffer overflow. But it
also looks like the code would be a lot simpler if you just used
getline() from glibc (and if you don't want to be tied to glibc, just
reimplement it and ideally push it to some other C library).
^ permalink raw reply [flat|nested] 17+ messages in thread
* configfile.c:63:14: warning: array subscript is above array [-Warray-bounds]
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
0 siblings, 1 reply; 17+ messages in thread
From: Jason @ 2013-06-04 10:34 UTC (permalink / raw)
Here is the function in question:
static int read_config_line(FILE *f, char *line, const char **value,
int bufsize)
{
int i = 0, isname = 0;
*value = NULL;
while (i < bufsize - 1) {
int c = next_char(f);
if (!isname && (c == '#' || c == ';')) {
skip_line(f);
continue;
}
if (!isname && isspace(c))
continue;
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++;
}
line[i + 1] = 0;
return i;
}
The ways out of this loop are when it encounters a \n or EOF after
finding a name, or when i == bufsize - 1. In the former case, an extra
null terminator is needlessly added. In the latter, we write a null
into line[bufsize], which, as you've pointed out, is out of bounds.
I'll adjust it to this:
static int read_config_line(FILE *f, char *line, const char **value,
int bufsize)
{
int i = 0, isname = 0;
*value = NULL;
while (i < bufsize - 1) {
int c = next_char(f);
if (!isname && (c == '#' || c == ';')) {
skip_line(f);
continue;
}
if (!isname && isspace(c))
continue;
if (c == '=' && !*value) {
line[i] = 0;
*value = &line[i + 1];
} else if (c == '\n' && !isname) {
i = 0;
continue;
} else if (c == '\n' || c == EOF) {
break;
} else {
line[i] = c;
}
isname = 1;
i++;
}
line[i] = 0;
return i;
}
Look good?
Thanks Kent.
^ permalink raw reply [flat|nested] 17+ messages in thread
* configfile.c:63:14: warning: array subscript is above array [-Warray-bounds]
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
0 siblings, 1 reply; 17+ messages in thread
From: cgit @ 2013-06-04 13:57 UTC (permalink / raw)
On Tue, Jun 04, 2013 at 12:34:49PM +0200, Jason A. Donenfeld wrote:
> Here is the function in question:
>
> [...]
>
> I'll adjust it to this:
>
> static int read_config_line(FILE *f, char *line, const char **value,
> int bufsize)
> {
> [...]
> } else if (c == '\n' || c == EOF) {
> break;
> } else {
> line[i] = c;
> }
> isname = 1;
> i++;
> }
> line[i] = 0;
> return i;
> }
>
> Look good?
Ack. Maybe we can further improve this by using strbuf_*() instead of
fixed-size buffers?
>
> Thanks Kent.
> _______________________________________________
> CGit mailing list
> CGit at lists.zx2c4.com
> http://lists.zx2c4.com/mailman/listinfo/cgit
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH] Use strbuf for reading configuration files
2013-06-04 13:57 ` cgit
@ 2013-06-04 14:47 ` cgit
2013-06-04 15:43 ` cgit
` (2 more replies)
0 siblings, 3 replies; 17+ messages in thread
From: cgit @ 2013-06-04 14:47 UTC (permalink / raw)
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>
---
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
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH] Use strbuf for reading configuration files
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 18:23 ` john
2013-06-05 11:17 ` [PATCH v2] " cgit
2 siblings, 1 reply; 17+ messages in thread
From: cgit @ 2013-06-04 15:43 UTC (permalink / raw)
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>
> ---
> 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)
> [...]
> + /* Skip comments and preceding spaces. */
> + while (c == '#' || c == ';') {
> + skip_line(f);
> + c = next_char(f);
> + }
> + while (isspace(c))
> + c = next_char(f);
Just noticed that this no longer detects indented comments. Not sure if
it makes sense to accept these (since we already only allow full
fill-line comments) but given that no longer allowing them is probably
considered a regression I will amend the patch and resubmit as far and
as soon as there aren't any other comments.
> +
> + /* Read variable name. */
> [...]
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH] Use strbuf for reading configuration files
2013-06-04 15:43 ` cgit
@ 2013-06-04 17:29 ` Jason
2013-06-04 17:35 ` Jason
2013-06-04 17:39 ` cgit
0 siblings, 2 replies; 17+ messages in thread
From: Jason @ 2013-06-04 17:29 UTC (permalink / raw)
Looks mostly okay. While you're at it, what about fixing the BUG mentioned
at the bottom of cgitrc.5.txt?
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.zx2c4.com/pipermail/cgit/attachments/20130604/e993f468/attachment.html>
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH] Use strbuf for reading configuration files
2013-06-04 17:29 ` Jason
@ 2013-06-04 17:35 ` Jason
2013-06-04 17:39 ` cgit
1 sibling, 0 replies; 17+ messages in thread
From: Jason @ 2013-06-04 17:35 UTC (permalink / raw)
It looks like we can also borrow routines directly from git, off of which
it appears config.c was initial based:
http://git.zx2c4.com/git/tree/config.c
In fact, what about reworking our config in general to use git's system?
This would involve supporting both our:
repo.blah = val
syntax as well as
[repo]
blah = val
which might be a little bit nice.
http://git.zx2c4.com/git/tree/config.c#n898
git_config_from_file might be a possibility.
We'd get nice escaping too.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.zx2c4.com/pipermail/cgit/attachments/20130604/77e78961/attachment.html>
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH] Use strbuf for reading configuration files
2013-06-04 17:29 ` Jason
2013-06-04 17:35 ` Jason
@ 2013-06-04 17:39 ` cgit
2013-06-04 17:40 ` Jason
1 sibling, 1 reply; 17+ messages in thread
From: cgit @ 2013-06-04 17:39 UTC (permalink / raw)
On Tue, Jun 04, 2013 at 07:29:07PM +0200, Jason A. Donenfeld wrote:
> Looks mostly okay. While you're at it, what about fixing the BUG mentioned at
> the bottom of cgitrc.5.txt?
Mh. I don't think that it is trivial to fix -- especially with regard to
backwards compatibility. In order to allow inline comments we would
probably have to add proper quoting (think about descriptions and file
names including "#") and break existing configurations. Any other ideas?
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH] Use strbuf for reading configuration files
2013-06-04 17:39 ` cgit
@ 2013-06-04 17:40 ` Jason
2013-06-04 17:49 ` Jason
0 siblings, 1 reply; 17+ messages in thread
From: Jason @ 2013-06-04 17:40 UTC (permalink / raw)
Implementing the git_config_parse_parameter as we speak! Standby.
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH] Use strbuf for reading configuration files
2013-06-04 17:40 ` Jason
@ 2013-06-04 17:49 ` Jason
0 siblings, 0 replies; 17+ messages in thread
From: Jason @ 2013-06-04 17:49 UTC (permalink / raw)
On Tue, Jun 4, 2013 at 7:40 PM, Jason A. Donenfeld <Jason at zx2c4.com> wrote:
> Implementing the git_config_parse_parameter as we speak! Standby.
Just sent some general sketch of what I have in mind if you'd like to try
your hand with it.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.zx2c4.com/pipermail/cgit/attachments/20130604/d636c89e/attachment.html>
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH] Use strbuf for reading configuration files
2013-06-04 14:47 ` [PATCH] Use strbuf for reading configuration files cgit
2013-06-04 15:43 ` cgit
@ 2013-06-04 18:23 ` john
2013-06-05 11:20 ` cgit
2013-06-05 11:17 ` [PATCH v2] " cgit
2 siblings, 1 reply; 17+ messages in thread
From: john @ 2013-06-04 18:23 UTC (permalink / raw)
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
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v2] Use strbuf for reading configuration files
2013-06-04 14:47 ` [PATCH] Use strbuf for reading configuration files cgit
2013-06-04 15:43 ` cgit
2013-06-04 18:23 ` john
@ 2013-06-05 11:17 ` cgit
2013-06-13 21:25 ` cgit
2 siblings, 1 reply; 17+ messages in thread
From: cgit @ 2013-06-05 11:17 UTC (permalink / raw)
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>
---
configfile.c | 64 +++++++++++++++++++++++++++++++-----------------------------
configfile.h | 2 ++
2 files changed, 35 insertions(+), 31 deletions(-)
diff --git a/configfile.c b/configfile.c
index d98989c..31fe5c8 100644
--- a/configfile.c
+++ b/configfile.c
@@ -31,45 +31,45 @@ 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;
+ /* Skip comments and preceding spaces. */
+ for(;;) {
+ if (c == '#' || c == ';')
+ skip_line(f);
+ else if (!isspace(c))
break;
- } else {
- line[i] = c;
- }
- isname = 1;
- i++;
+ c = next_char(f);
}
- line[i + 1] = 0;
- return i;
+
+ /* Read variable name. */
+ while (c != '=') {
+ if (c == '\n' || c == EOF)
+ return 0;
+ strbuf_addch(name, c);
+ c = next_char(f);
+ }
+
+ /* 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 +78,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
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH] Use strbuf for reading configuration files
2013-06-04 18:23 ` john
@ 2013-06-05 11:20 ` cgit
0 siblings, 0 replies; 17+ messages in thread
From: cgit @ 2013-06-05 11:20 UTC (permalink / raw)
On Tue, Jun 04, 2013 at 07:23:03PM +0100, John Keeping wrote:
> 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?
* We would need to deal with CRLF separately (check what next_char()
currently does).
* I don't think it is much shorter or significantly improves
readability.
* It might be a tad slower (we need to look for the delimiter after
reading the whole line vs. doing it on the fly).
* Git uses strbuf_addch() in their configuration file parser as well
(although their format is a tad more complex and there might be other
reasons for using it there).
I will resubmit v2 that still uses strbuf_addch() -- feel free to submit
an improved version that uses strbuf_getline() and
string_list_split_in_place() :)
>
> > 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
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v2] Use strbuf for reading configuration files
2013-06-05 11:17 ` [PATCH v2] " cgit
@ 2013-06-13 21:25 ` cgit
2013-08-02 17:13 ` cgit
0 siblings, 1 reply; 17+ messages in thread
From: cgit @ 2013-06-13 21:25 UTC (permalink / raw)
On Wed, Jun 05, 2013 at 01:17:00PM +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>
> ---
> configfile.c | 64 +++++++++++++++++++++++++++++++-----------------------------
> configfile.h | 2 ++
> 2 files changed, 35 insertions(+), 31 deletions(-)
>
Status...?
> [...]
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v2] Use strbuf for reading configuration files
2013-06-13 21:25 ` cgit
@ 2013-08-02 17:13 ` cgit
2013-08-02 18:14 ` Jason
2013-08-12 18:52 ` Jason
0 siblings, 2 replies; 17+ messages in thread
From: cgit @ 2013-08-02 17:13 UTC (permalink / raw)
On Thu, Jun 13, 2013 at 11:25:53PM +0200, Lukas Fleischer wrote:
> On Wed, Jun 05, 2013 at 01:17:00PM +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>
> > ---
> > configfile.c | 64 +++++++++++++++++++++++++++++++-----------------------------
> > configfile.h | 2 ++
> > 2 files changed, 35 insertions(+), 31 deletions(-)
> >
>
> Status...?
*bump*
I put all pending patches on the "for-jason" branch of my public cgit
repository [1], so it shouldn't take much longer than 20 seconds to
merge them into master...
>
> > [...]
> _______________________________________________
> CGit mailing list
> CGit at lists.zx2c4.com
> http://lists.zx2c4.com/mailman/listinfo/cgit
[1] http://git.cryptocrack.de/cgit.git/log/?h=for-jason
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v2] Use strbuf for reading configuration files
2013-08-02 17:13 ` cgit
@ 2013-08-02 18:14 ` Jason
2013-08-12 18:52 ` Jason
1 sibling, 0 replies; 17+ messages in thread
From: Jason @ 2013-08-02 18:14 UTC (permalink / raw)
Thanks for consolidating things Lukas. I'm still on vacation (til
mid-August) but I'll be getting to it asap.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.zx2c4.com/pipermail/cgit/attachments/20130802/f8779f0b/attachment.html>
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v2] Use strbuf for reading configuration files
2013-08-02 17:13 ` cgit
2013-08-02 18:14 ` Jason
@ 2013-08-12 18:52 ` Jason
1 sibling, 0 replies; 17+ messages in thread
From: Jason @ 2013-08-12 18:52 UTC (permalink / raw)
Merged at last! Thanks Lukas.
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2013-08-12 18:52 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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
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
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).