xfs
[Top] [All Lists]

Re: [PATCH v2 4/4] xfsprogs: OS X partial support

To: Jan Tulak <jtulak@xxxxxxxxxx>
Subject: Re: [PATCH v2 4/4] xfsprogs: OS X partial support
From: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Date: Sun, 26 Jul 2015 09:34:38 -0700
Cc: xfs@xxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <1437397018-12864-5-git-send-email-jtulak@xxxxxxxxxx>
References: <1437397018-12864-1-git-send-email-jtulak@xxxxxxxxxx> <1437397018-12864-5-git-send-email-jtulak@xxxxxxxxxx>
User-agent: Mutt/1.5.23 (2014-03-12)
I think this needs to be split into one patch per issue.  A few comments
below:

> -#include <malloc.h>


malloc and friends need to be in stdlib.h per Posix, so the right
fix is to drop every include of malloc.h and mae sure we include
stdlib.h where needed.

> -#include <mntent.h>
>  #include <syslog.h>
>  #include <signal.h>
>  #include <sys/ioctl.h>
>  #include <sys/wait.h>
> -#include <sys/vfs.h>

sys/vfs.h is jsut an alias for sys/stafs.h, ut given that
this file doesn't even use statfs we can simply drop the include
unconditionally.

> +#ifndef PLATFORM_HAS_NO_MNTENT_H
> +#  include <mntent.h>
> +#endif

Why aren't you using the standard autoconf-generate include checks?

> +#ifdef _DIRENT_HAVE_D_RECLEN
>               *total += dirent->d_reclen;
> +#endif
>               count++;
>  
>               if (dump) {
>                       dump_dirent(offset, dirent);
> +#ifdef _DIRENT_HAVE_D_OFF
>                       offset = dirent->d_off;
> +#endif

I'm pretty sure autoconf has a standard macro for this, try looking it
up in the autoconf manual.

> diff --git a/libxcmd/paths.c b/libxcmd/paths.c
> index c0b1ddb..b360617 100644
> --- a/libxcmd/paths.c
> +++ b/libxcmd/paths.c
> @@ -364,7 +364,7 @@ fs_table_initialise_mounts(
>                       continue;
>               if (!realpath(stats[i].f_mntfromname, rmntfromname))
>                       continue;
> -             if (!realpath(stats[i].f_mntonname, rmnttomname)))
> +             if (!realpath(stats[i].f_mntonname, rmntonname))

I don't understand this change.

<Prev in Thread] Current Thread [Next in Thread>