Re: [Minios-devel] [UNIKRAFT VFS PATCH 2/3] Check for NULL reference on vfscore_put_fd(). Add asserts to fdrop()

[Costin Lupu <costin.lup@xxxxxxxxx>]

Hi Mihai,

As a general rule, we skip adding commit messages when the commits are
small enough and self explanatory, almost trivial. This is not the case
for the current patch.

We have 2 patches in 1 here. The first one checks for NULL before
dropping the reference and it should have a commit message which may
repeat the idea from the comment (not necessarily with the same words,
maybe a bit more detailed). The second patch should actually be about
decrementing the refcount when dropping reference for vfscore file
abstraction.

Both patches should have commit messages with explanation about the changes.

On 6/26/19 4:54 AM, Mihai Pogonaru wrote:
> Signed-off-by: Mihai Pogonaru <pogonarumihai@xxxxxxxxx>
> ---
>  lib/vfscore/fd.c   | 7 ++++++-
>  lib/vfscore/file.c | 7 ++++++-
>  2 files changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/vfscore/fd.c b/lib/vfscore/fd.c
> index f8b24020..01025a46 100644
> --- a/lib/vfscore/fd.c
> +++ b/lib/vfscore/fd.c
> @@ -88,7 +88,12 @@ void vfscore_put_fd(int fd)
>       fdtable.files[fd] = NULL;
>       ukplat_lcpu_restore_irqf(flags);
>  
> -     fdrop(fp);
> +     /*
> +      * Since we can alloc a fd without assigning a
> +      * vfsfile we must protect against NULL ptr
> +      */
> +     if (fp)
> +             fdrop(fp);
>  }
>  
>  int vfscore_install_fd(int fd, struct vfscore_file *file)
> diff --git a/lib/vfscore/file.c b/lib/vfscore/file.c
> index e7627e96..bc8b0dc8 100644
> --- a/lib/vfscore/file.c
> +++ b/lib/vfscore/file.c
> @@ -42,7 +42,12 @@
>  
>  int fdrop(struct vfscore_file *fp)
>  {
> -     int prev = ukarch_dec(&fp->f_count);
> +     int prev;
> +
> +     UK_ASSERT(fp);
> +     UK_ASSERT(fp->f_count > 0);
> +
> +     prev = ukarch_dec(&fp->f_count);
>  
>       if (prev == 0)
>               UK_CRASH("Unbalanced fhold/fdrop");
> 

_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel