emacs-devel
[Top][All Lists]
Advanced

[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



reply via email to

[Prev in Thread] Current Thread [Next in Thread]