Re: simlpe patch implements eval command (with printf-like format and args)

[Michael Snyder <msnyder@specifix.com>]

On Thu, 2008-02-21 at 11:20 +0200, Yakov Lerner wrote:
> This simple patch implements eval command with printf-like syntax:
> 
>         eval "printf-like-format", comma-separated args
> 
> The patch is  against cvs-checkedout source. Suggestions are welcome.
> Implementation is very simple.

Not bad at all, for a first contribution.  But I suggest 
you try to limit the scope a little bit and make it still
simpler -- maybe have it accept only strings, floats and ints
to start -- and see if you can't implement it less intrusively
eg. without taking over the function for another command and
introducing new cross-module infrastructure.  Then we can 
maybe iteratively improve on it.

I'll give you some style and structure feedback on this one:


> 
> +/* Grow allocated buffer and appens to it, without causing quadratic
> slowdown */

"append", of course -- and comments must be punctuated (in this case, 
period followed by two spaces".  Plus indentation -- consult the GNU
coding standard.


> +/* Grow allocated buffer and appens to it, without causing quadratic
> slowdown */

As above.  

> +static void
> +eval_command (char *arg, int from_tty)
> +{
> +  struct cleanup *old_cleanups;
> +  char *str = gdb_own_xasprintf (arg);
> +
> +  old_cleanups = make_cleanup (free_current_contents, &str);
> +  execute_command (str, from_tty);
> +  do_cleanups (old_chain);
> +}

old_chain is undefined.  Probably should be "old_cleanups"?

Actually, except for the comment tails, you've done a commendable
job of following the coding standard.  I'd like to see an implementation
that's limited enough not to require the "append" function.

Thanks,
Michael