[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: Patch for fields of `struct buffer'
From: |
Stefan Monnier |
Subject: |
Re: Patch for fields of `struct buffer' |
Date: |
Tue, 08 Feb 2011 16:02:58 -0500 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.0.50 (gnu/linux) |
> === modified file 'configure'
> --- configure 2011-02-05 22:30:14 +0000
> +++ configure 2011-02-08 14:54:38 +0000
It's generally better to skip "configure" (and other generated files) when
you pose a patch for review. Luckily in this case, the changes in
"configure" were small.
> +/* A single global. */
> +struct global
> +{
> + char type;
> + char *name;
> +};
Please use an enum type rather than a char for the "type" field.
> +static void
> +add_global (char type, char *name)
> +{
> + /* Ignore the one non-symbol that can occur. */
> + if (strcmp (name, "..."))
> + {
> + ++num_globals;
> + globals = xrealloc (globals, num_globals * sizeof (struct global));
> + globals[num_globals - 1].type = type;
> + globals[num_globals - 1].name = name;
> + }
> +}
I'd have used a singly-linked list or a "xrealloc (twice the size)"
scheme, to avoid allocating N^2 amount of memory. But I guess it
doesn't matter for make-docfile.
> + qsort (globals, num_globals, sizeof (struct global), compare_globals);
Good.
> + if (globals[i].type == 'I')
> + type = "EMACS_INT";
> + else if (globals[i].type == 'B')
> + type = "int";
> + else if (globals[i].type == 'L')
> + type = "Lisp_Object";
> + else
> + fatal ("not a recognized DEFVAR_", 0);
Better use `switch'.
> + if (generate_globals && (!defvarflag || defvarperbufferflag
> + || (type != 'I' && type != 'B'
> + && type != 'L')))
> + continue;
Why test (type != 'I' && type != 'B' && type != 'L')?
The rest looks good, thank you.
Stefan
- Re: Patch for fields of `struct buffer', (continued)
Re: Patch for fields of `struct buffer', Andreas Schwab, 2011/02/01
Re: Patch for fields of `struct buffer', Tom Tromey, 2011/02/01
Re: Patch for fields of `struct buffer', Tom Tromey, 2011/02/08
Re: Patch for fields of `struct buffer',
Stefan Monnier <=
Re: Patch for fields of `struct buffer', Tom Tromey, 2011/02/08
Re: Patch for fields of `struct buffer', Tom Tromey, 2011/02/08
Re: Patch for fields of `struct buffer', Stefan Monnier, 2011/02/09
Re: Patch for fields of `struct buffer', Andreas Schwab, 2011/02/08
Re: Patch for fields of `struct buffer', Stefan Monnier, 2011/02/01
Re: Patch for fields of `struct buffer', Juanma Barranquero, 2011/02/02