RE: [PATCH v2 3/3] disk: read into cache directly

[Elliott, Robert (Persistent Memory)]

> -----Original Message-----
> From: address@hidden [mailto:grub-devel-
> address@hidden On Behalf Of Leif Lindholm
> Sent: Tuesday, March 01, 2016 6:14 PM
> To: address@hidden
> Subject: [PATCH v2 3/3] disk: read into cache directly
> 
> From: Andrei Borzenkov <address@hidden>
> 
...
>  void
> +grub_disk_cache_free (struct grub_cache_buffer *buf)
> +{
> +  if (!buf->count)
> +    /* FIXME This means corruption; what can we do? */
> +    return;
> +
> +  if (!--buf->count)
> +    {
> +      grub_free (buf->data);
> +      grub_free (buf);
> +    }
> +}
...
> -      if (cache->data && ! cache->lock)
> +      if (cache->buffer && ! cache->lock)
>       {
> -       grub_free (cache->data);
> -       cache->data = 0;
> +       grub_disk_cache_free (cache->buffer);
> +       cache->buffer = 0;
...
> +      if (cache->buffer)
> +     grub_disk_cache_free (cache->buffer);

It's pretty common for free() functions to do NULL pointer
checks so the caller doesn't have to; grub_free() is one
such example.  That makes cleanup code a bit safer.  You 
might want to follow that pattern for this function.


---
Robert Elliott, HPE Persistent Memory