On Sat, Jul 01, 2017 at 08:48:18PM -0400, Bryan Steele wrote:
> On Sat, Jul 01, 2017 at 06:53:29PM -0400, Bryan Steele wrote:
> > On Sun, Jul 02, 2017 at 12:26:19AM +0200, Ingo Schwarze wrote:
> > > I don't see a need for two char * variables here, that's an artifact
> > > of the typical fgetln(3) complications. Just
> > >
> > > char *line = NULL;
> > > size_t linesize = 0;
> > > ssize_t slen;
> > >
> > > while ((slen = getline(&line, &linesize, f)) != -1) {
> > > if (line[slen - 1] == '\n')
> > > /* etc. */
> > > }
> > > free(line);
> > > if (ferror(f))
> > > err(1, "%s", path);
> > >
> > > ought to suffice.
> > >
> > > Yours,
> > > Ingo
> >
> > Hmm, there's a Segmentation fault that I can't seems to narrow
> > down with the change you suggest. That's odd..
>
> This appears to be because the loop passes around the line pointer
> and increments it, fixing that would be a lager diff.. but I'd
> still appreciate a cluestick if I missed something.
>
> -Bryan.
Here's the final diff incorporating all the feedback recieved, except
for eliminating the tmp variable as it is used to keep track of the
getline pointer which gets munged each iteration.
-Bryan.
Index: magic-load.c
===================================================================
RCS file: /cvs/src/usr.bin/file/magic-load.c,v
retrieving revision 1.25
diff -u -p -r1.25 magic-load.c
--- magic-load.c 1 Jul 2017 14:34:29 -0000 1.25
+++ magic-load.c 2 Jul 2017 04:58:52 -0000
@@ -19,6 +19,7 @@
#include <sys/types.h>
#include <ctype.h>
+#include <err.h>
#include <errno.h>
#include <limits.h>
#include <regex.h>
@@ -1071,6 +1072,7 @@ magic_load(FILE *f, const char *path, in
struct magic_line *ml = NULL, *parent, *parent0;
char *line, *tmp;
size_t size;
+ ssize_t slen;
u_int at, level, n, i;
m = xcalloc(1, sizeof *m);
@@ -1084,15 +1086,12 @@ magic_load(FILE *f, const char *path, in
at = 0;
tmp = NULL;
- while ((line = fgetln(f, &size))) {
- if (line[size - 1] == '\n')
- line[size - 1] = '\0';
- else {
- tmp = xmalloc(size + 1);
- memcpy(tmp, line, size);
- tmp[size] = '\0';
- line = tmp;
- }
+ size = 0;
+ while ((slen = getline(&tmp, &size, f)) != -1) {
+ line = tmp;
+ if (line[slen - 1] == '\n')
+ line[slen - 1] = '\0';
+
at++;
while (isspace((u_char)*line))
@@ -1168,6 +1167,8 @@ magic_load(FILE *f, const char *path, in
parent0 = ml;
}
free(tmp);
+ if (ferror(f))
+ err(1, "%s", path);
return (m);
}
