Re: file(1): prevent printing unknown magic filename
Now thinking again I see no much point in my diff, I prefer yours as
it's easier to update to a newer file(1), no point in changing only
this.
ok from me.
On Wed, Sep 28, 2011 at 12:33:13AM -0300, Christiano F. Haesbaert wrote:
> On Thu, Sep 22, 2011 at 03:56:11PM +0100, Edd Barrett wrote:
> > Hi,
> >
> > An upstream of one of the ports I work on (radare2) has imported our file(1)
> > implementation and claims to have found bugs. This is the first:
> >
> > 'ms->file' will only be assigned to 'fn' after a call to apprentice_load()
> > and
> > ultimately load_1(), so the file_magwarn() at line 274 would report the
> > default
> > filename "unknown".
> >
> > We can trigger this behaviour by executing `file -c`:
> > unknown, 0: Warning: using regular magic file `/etc/magic'
> >
> > Is it a bug?
> >
> > Index: apprentice.c
> > ===
> > RCS file: /cvs/src/usr.bin/file/apprentice.c,v
> > retrieving revision 1.29
> > diff -u -r1.29 apprentice.c
> > --- apprentice.c11 Nov 2009 16:21:51 - 1.29
> > +++ apprentice.c22 Sep 2011 14:27:17 -
> > @@ -258,6 +258,7 @@
> > return -1;
> > }
> >
> > + ms->file = fn;
> > if (action == FILE_COMPILE) {
> > rv = apprentice_load(ms, &magic, &nmagic, fn, action);
> > if (rv != 0)
>
> Seems correct to me, but this "fn" seems kinda redundant down the
> stack, except for load_1() which really needs a file and not a
> directory.
>
> So I thought we could kill this fn by assigning ms->file right in the
> beginning at file_apprentice(). Also tweak load_1() so it won't do the
> whole processing in an else.
>
> Index: apprentice.c
> ===
> RCS file: /cvs/src/usr.bin/file/apprentice.c,v
> retrieving revision 1.29
> diff -d -u -p -w -r1.29 apprentice.c
> --- apprentice.c 11 Nov 2009 16:21:51 - 1.29
> +++ apprentice.c 28 Sep 2011 03:18:29 -
> @@ -96,11 +96,11 @@ private int parse(struct magic_set *, st
> private int parse_mime(struct magic_set *, struct magic_entry **, uint32_t *,
> const char *);
> private void eatsize(const char **);
> -private int apprentice_1(struct magic_set *, const char *, int, struct mlist
> *);
> +private int apprentice_1(struct magic_set *, int, struct mlist *);
> private size_t apprentice_magic_strength(const struct magic *);
> private int apprentice_sort(const void *, const void *);
> private int apprentice_load(struct magic_set *, struct magic **, uint32_t *,
> -const char *, int);
> +int);
> private void byteswap(struct magic *, uint32_t);
> private void bs1(struct magic *);
> private uint16_t swap2(uint16_t);
> @@ -242,7 +242,7 @@ init_file_tables(void)
> * Handle one file or directory.
> */
> private int
> -apprentice_1(struct magic_set *ms, const char *fn, int action,
> +apprentice_1(struct magic_set *ms, int action,
> struct mlist *mlist)
> {
> struct magic *magic = NULL;
> @@ -259,19 +259,19 @@ apprentice_1(struct magic_set *ms, const
> }
>
> if (action == FILE_COMPILE) {
> - rv = apprentice_load(ms, &magic, &nmagic, fn, action);
> + rv = apprentice_load(ms, &magic, &nmagic, action);
> if (rv != 0)
> return -1;
> - rv = apprentice_compile(ms, &magic, &nmagic, fn);
> + rv = apprentice_compile(ms, &magic, &nmagic, ms->file);
> free(magic);
> return rv;
> }
>
> #ifndef COMPILE_ONLY
> - if ((rv = apprentice_map(ms, &magic, &nmagic, fn)) == -1) {
> + if ((rv = apprentice_map(ms, &magic, &nmagic, ms->file)) == -1) {
> if (ms->flags & MAGIC_CHECK)
> - file_magwarn(ms, "using regular magic file `%s'", fn);
> - rv = apprentice_load(ms, &magic, &nmagic, fn, action);
> + file_magwarn(ms, "using regular magic file `%s'",
> ms->file);
> + rv = apprentice_load(ms, &magic, &nmagic, action);
> if (rv != 0)
> return -1;
> }
> @@ -359,7 +359,8 @@ file_apprentice(struct magic_set *ms, co
> *p++ = '\0';
> if (*fn == '\0')
> break;
> - file_err = apprentice_1(ms, fn, action, mlist);
> + ms->file = fn;
> + file_err = apprentice_1(ms, action, mlist);
> errs = MAX(errs, file_err);
> fn = p;
> }
> @@ -571,13 +572,15 @@ load_1(struct magic_set *ms, int action,
> {
> char line[BUFSIZ];
> size_t lineno = 0;
> - FILE *f = fopen(ms->file = fn, "r");
> - if (f == NULL) {
> + FILE *f;
> +
> + if ((f = fopen(fn, "r")) == NULL) {
> if (errno != ENOENT)
> file_error(ms, errno, "cannot read magic file `%s'",
> fn);
> (*errs)++;
Re: file(1): prevent printing unknown magic filename
On Thu, Sep 22, 2011 at 03:56:11PM +0100, Edd Barrett wrote:
> Hi,
>
> An upstream of one of the ports I work on (radare2) has imported our file(1)
> implementation and claims to have found bugs. This is the first:
>
> 'ms->file' will only be assigned to 'fn' after a call to apprentice_load() and
> ultimately load_1(), so the file_magwarn() at line 274 would report the
> default
> filename "unknown".
>
> We can trigger this behaviour by executing `file -c`:
> unknown, 0: Warning: using regular magic file `/etc/magic'
>
> Is it a bug?
>
> Index: apprentice.c
> ===
> RCS file: /cvs/src/usr.bin/file/apprentice.c,v
> retrieving revision 1.29
> diff -u -r1.29 apprentice.c
> --- apprentice.c 11 Nov 2009 16:21:51 - 1.29
> +++ apprentice.c 22 Sep 2011 14:27:17 -
> @@ -258,6 +258,7 @@
> return -1;
> }
>
> + ms->file = fn;
> if (action == FILE_COMPILE) {
> rv = apprentice_load(ms, &magic, &nmagic, fn, action);
> if (rv != 0)
Seems correct to me, but this "fn" seems kinda redundant down the
stack, except for load_1() which really needs a file and not a
directory.
So I thought we could kill this fn by assigning ms->file right in the
beginning at file_apprentice(). Also tweak load_1() so it won't do the
whole processing in an else.
Index: apprentice.c
===
RCS file: /cvs/src/usr.bin/file/apprentice.c,v
retrieving revision 1.29
diff -d -u -p -w -r1.29 apprentice.c
--- apprentice.c11 Nov 2009 16:21:51 - 1.29
+++ apprentice.c28 Sep 2011 03:18:29 -
@@ -96,11 +96,11 @@ private int parse(struct magic_set *, st
private int parse_mime(struct magic_set *, struct magic_entry **, uint32_t *,
const char *);
private void eatsize(const char **);
-private int apprentice_1(struct magic_set *, const char *, int, struct mlist
*);
+private int apprentice_1(struct magic_set *, int, struct mlist *);
private size_t apprentice_magic_strength(const struct magic *);
private int apprentice_sort(const void *, const void *);
private int apprentice_load(struct magic_set *, struct magic **, uint32_t *,
-const char *, int);
+int);
private void byteswap(struct magic *, uint32_t);
private void bs1(struct magic *);
private uint16_t swap2(uint16_t);
@@ -242,7 +242,7 @@ init_file_tables(void)
* Handle one file or directory.
*/
private int
-apprentice_1(struct magic_set *ms, const char *fn, int action,
+apprentice_1(struct magic_set *ms, int action,
struct mlist *mlist)
{
struct magic *magic = NULL;
@@ -259,19 +259,19 @@ apprentice_1(struct magic_set *ms, const
}
if (action == FILE_COMPILE) {
- rv = apprentice_load(ms, &magic, &nmagic, fn, action);
+ rv = apprentice_load(ms, &magic, &nmagic, action);
if (rv != 0)
return -1;
- rv = apprentice_compile(ms, &magic, &nmagic, fn);
+ rv = apprentice_compile(ms, &magic, &nmagic, ms->file);
free(magic);
return rv;
}
#ifndef COMPILE_ONLY
- if ((rv = apprentice_map(ms, &magic, &nmagic, fn)) == -1) {
+ if ((rv = apprentice_map(ms, &magic, &nmagic, ms->file)) == -1) {
if (ms->flags & MAGIC_CHECK)
- file_magwarn(ms, "using regular magic file `%s'", fn);
- rv = apprentice_load(ms, &magic, &nmagic, fn, action);
+ file_magwarn(ms, "using regular magic file `%s'",
ms->file);
+ rv = apprentice_load(ms, &magic, &nmagic, action);
if (rv != 0)
return -1;
}
@@ -359,7 +359,8 @@ file_apprentice(struct magic_set *ms, co
*p++ = '\0';
if (*fn == '\0')
break;
- file_err = apprentice_1(ms, fn, action, mlist);
+ ms->file = fn;
+ file_err = apprentice_1(ms, action, mlist);
errs = MAX(errs, file_err);
fn = p;
}
@@ -571,13 +572,15 @@ load_1(struct magic_set *ms, int action,
{
char line[BUFSIZ];
size_t lineno = 0;
- FILE *f = fopen(ms->file = fn, "r");
- if (f == NULL) {
+ FILE *f;
+
+ if ((f = fopen(fn, "r")) == NULL) {
if (errno != ENOENT)
file_error(ms, errno, "cannot read magic file `%s'",
fn);
(*errs)++;
- } else {
+ return;
+ }
/* read and parse this file */
for (ms->line = 1; fgets(line, sizeof(line), f) != NULL;
ms->line++) {
size_t len;
@@ -606,7 +609,6 @@ load_1(struct magic_set *ms, int action,
(void)fclose(f);
}
-}
/*
* parse a file or directory of f
file(1): prevent printing unknown magic filename
Hi,
An upstream of one of the ports I work on (radare2) has imported our file(1)
implementation and claims to have found bugs. This is the first:
'ms->file' will only be assigned to 'fn' after a call to apprentice_load() and
ultimately load_1(), so the file_magwarn() at line 274 would report the default
filename "unknown".
We can trigger this behaviour by executing `file -c`:
unknown, 0: Warning: using regular magic file `/etc/magic'
Is it a bug?
Index: apprentice.c
===
RCS file: /cvs/src/usr.bin/file/apprentice.c,v
retrieving revision 1.29
diff -u -r1.29 apprentice.c
--- apprentice.c11 Nov 2009 16:21:51 - 1.29
+++ apprentice.c22 Sep 2011 14:27:17 -
@@ -258,6 +258,7 @@
return -1;
}
+ ms->file = fn;
if (action == FILE_COMPILE) {
rv = apprentice_load(ms, &magic, &nmagic, fn, action);
if (rv != 0)
--
Best Regards
Edd Barrett
http://www.theunixzoo.co.uk
