Re: [PATCH 3/3] Add a pair of system calls to make extended file stats available [ver #2]

[Arnd Bergmann]

On Wednesday 30 June 2010 03:17:12 David Howells wrote:
> +static int xstat_check_param(struct xstat __user *buffer, size_t bufsize,
> +                            struct kstat *stat)
> +{
> +       u32 struct_version;
> +       int ret;
> +
> +       /* if the buffer isn't large enough, return how much we wanted to
> +        * write, but otherwise do nothing */
> +       if (bufsize < sizeof(struct xstat))
> +               return sizeof(struct xstat);
> +
> +       ret = get_user(struct_version, &buffer->struct_version);
> +       if (ret < 0)
> +               return ret;
> +       if (struct_version != 0)
> +               return -ENOTSUPP;
> +
> +       memset(stat, 0xde, sizeof(*stat));
> +
> +       ret = get_user(stat->query_flags, &buffer->query_flags);
> +       if (ret < 0)
> +               return ret;
> +
> +       /* nothing outside this set has a defined purpose */
> +       stat->query_flags &= XSTAT_QUERY__DEFINED_SET;
> +       stat->result_flags = 0;
> +       return 0;
> +}

I think it would be better to leave the structure as write-only from
the kernel and pass the query_flags and struct_version as syscall
arguments, though it makes sense to store them in the result as well.

Independent from this, I also think that we can collapse the
struct_version into the more flexible query_flags. When the structure
gets extended with new fields, just add another flag to let
the user ask for them.

When the flags are outside of the structure, you can even have a flag
that will result in a completely new structure layout to be returned.

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/