Re: [PATCH] Refactor commands/search.c

[Robert Millan <rmh@aybabtu.com>]

On Mon, Jul 06, 2009 at 05:13:03PM -0400, Pavel Roskin wrote:
> ChangeLog:
> 
> 	* commands/search.c (search_file): Merge into ...
> 	(search_fs): ... this.  On error, don't output what we were
> 	looking for.  Accept search type as argument.
> 	(grub_cmd_search): Pass search type to search_fs().

Very nice work.  I've been wanting to do this for a while...

> +    if (type == SEARCH_FILE)
> +      {
> +	grub_size_t len;
> +	char *p;
> +	grub_file_t file;
> +
> +	len = grub_strlen (name) + 2 + grub_strlen (key) + 1;
> +	p = grub_realloc (buf, len);
> +	if (! p)
> +	  return 1;
> +
> +	buf = p;
> +	grub_sprintf (buf, "(%s)%s", name, key);
> +
> +	file = grub_file_open (buf);
> +	if (file)
> +	  {
> +	    found = 1;
> +	    grub_file_close (file);
> +	  }
> +      }
> +    else
> +      {

I'd suggest making this clearer by eithe explicitly checking for
SEARCH_XXX or otherwise put a comment after the else.

-- 
Robert Millan

  The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
  how) you may access your data; but nobody's threatening your freedom: we
  still allow you to remove your data and not access it at all."