Re: RFC: [PATCH] Re: Test case for fakeroot-hurd failure with a socket

[Samuel Thibault]

Hello,

Svante Signell, le Tue 12 May 2015 22:09:33 +0200, a écrit :
> netfs_attempt_chmod() returns an error of EOPNOTSUPP and hits the
> fallback case calling the stub in
> libnetfs/set-get-trans.c:netfs_set_translator() returning EOPNOTSUPP.
> 
> Implementing netfs_S_file_set_translator() calling file_set_translator()
> in trans/fakeroot.c overrides this call and solves the problem.

Err, no, see what I wrote earlier: “implement the
netfs_set_translator stub by just calling file_set_translator on the
underlying node to set the passive translator.” Do not overwrite
netfs_S_file_set_translator, it'd disable everything that is done in the
netfs_S_file_set_translator provided by libnetfs...

> - Are the checks in the beginning really needed doesn't
> file_set_translator() take care of that?

It seems you are seeing things backwards. It's libnetfs'
netfs_S_file_set_translator which is called first, does checks, and
calls file_set_translator, which you'd implement in fakeroot. As another
example, see libnetfs' netfs_S_dir_rmdir which does checks, and calls
netfs_attempt_rmdir, which in fakeroot.c just calls dir_rmdir on the
underlying node.

> - Is the explanation really OK?

Yes, it's the sort of things a reviewer needs, to be able to answer you
promptly without spending hours trying to figure out what you tried to
do.

> +netfs_S_file_set_translator (struct protid *user,
> +                          int passive_flags, int active_flags,
> +                          int killtrans_flags, char *passive,
> +                          mach_msg_type_number_t passivelen,
> +                          mach_port_t active)
> +{
...
> +
> +  if (passivelen && passive[passivelen - 1])
> +    return EINVAL;
> +
> +  char trans[sizeof _HURD_IFSOCK + passivelen];
> +  memcpy (trans, _HURD_IFSOCK, sizeof _HURD_IFSOCK);
> +  memcpy (&trans[sizeof _HURD_IFSOCK], passive, passivelen);

Err, no, don't build the translator path by hand, it's already given to
you in the "passive" parameter! Simply pass passive and passivelen to
file_set_translator.

Samuel