--AKkMM/tm2Mk6Yn/s
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

Hi Jeremy,

Jeremy Allison wrote:
> commit 4221937b68e2414295279b27c5f12a80f826ed4b
> Author: Jeremy Allison
> Date: Fri May 9 11:14:45 2008 -0700
>=20
> Remove a couple of uses of SMB_VFS_GET_NT_ACL(), use
> SMB_VFS_FGET_NT_ACL instead. I'd like to ultimately
> remove SMB_VFS_GET_NT_ACL.
> Jeremy.


Why?

I had some months ago put some effort in separating
the posix implementations of get_net_acl and fget_nt_acl,
removing the fsp part from the common code portion
and removing the fsp parameter from the SMB_VFS_GET_NT_ACL.

At that time you liked that, and I actually had the plan to
do it for set_nt_acl_too ... :-) I'd like to understand your
motivation in removing the non-F-variants:

> -----------------------------------------------------------------------
>=20
> diff --git a/source/rpc_server/srv_srvsvc_nt.c b/source/rpc_server/srv_sr=

vsvc_nt.c
> index 18c6f4d..947ad46 100644
> --- a/source/rpc_server/srv_srvsvc_nt.c
> +++ b/source/rpc_server/srv_srvsvc_nt.c
> ...
> - nt_status =3D SMB_VFS_GET_NT_ACL(conn, filename,
> + if (!(S_ISDIR(st.st_mode))) {
> + nt_status =3D open_file_ntcreate(conn, NULL, filename, &st,
> + FILE_READ_ATTRIBUTES,
> + FILE_SHARE_READ|FILE_SHARE_WRITE,
> + FILE_OPEN,
> + 0,
> + FILE_ATTRIBUTE_NORMAL,
> + 0,
> + NULL, &fsp);
> +
> + } else {
> + nt_status =3D open_directory(conn, NULL, filename, &st,
> + FILE_READ_ATTRIBUTES,
> + FILE_SHARE_READ|FILE_SHARE_WRITE,
> + FILE_OPEN,
> + 0,
> + FILE_ATTRIBUTE_DIRECTORY,
> + NULL, &fsp);
> + }
> +
> + if (!NT_STATUS_IS_OK(nt_status)) {
> + DEBUG(3,("_srvsvc_NetGetFileSecurity: can't open %s\n",
> + filename));
> + werr =3D ntstatus_to_werror(nt_status);
> + goto error_exit;
> + }
> +
> + nt_status =3D SMB_VFS_FGET_NT_ACL(fsp,
> (OWNER_SECURITY_INFORMATION
> |GROUP_SECURITY_INFORMATION
> |DACL_SECURITY_INFORMATION), &psd);


What is the benefit here?

Look at posix_fget_nt_acl(): in case of a directory, it calls
posix_get_nt_acl(). Otherwise an additional stat call which
would use the stat cache is replaces by an open_fil_ntcreate().

So it cannot possibly be performance?

> diff --git a/source/smbd/nttrans.c b/source/smbd/nttrans.c
> index 362823d..bd34b5a 100644
> --- a/source/smbd/nttrans.c
> +++ b/source/smbd/nttrans.c
> @@ -1612,14 +1612,8 @@ static void call_nt_transact_query_security_desc(c=

onnection_struct *conn,
> if (!lp_nt_acl_support(SNUM(conn))) {
> status =3D get_null_nt_acl(talloc_tos(), &psd);
> } else {
> - if (fsp->fh->fd !=3D -1) {
> - status =3D SMB_VFS_FGET_NT_ACL(
> - fsp, security_info_wanted, &psd);
> - }
> - else {
> - status =3D SMB_VFS_GET_NT_ACL(
> - conn, fsp->fsp_name, security_info_wanted, &psd);
> - }
> + status =3D SMB_VFS_FGET_NT_ACL(
> + fsp, security_info_wanted, &psd);
> }
> =20
> if (!NT_STATUS_IS_OK(status)) {


For the posix implementation, this just removes a double check:
beginning of posix_fget_nt_acl(), this will hit the following code.

=2E..
if (fsp->is_directory || fsp->fh->fd =3D=3D -1) {
return posix_get_nt_acl(fsp->conn, fsp->fsp_name,
security_info, ppdesc);
}
=2E..

For other implementations, may it not even fail if fd =3D=3D -1 ?

I'd really like to understand your motivation in removing the
non-F-variants of the GET/SET_NT_ACL calls!

Cheers - Michael

--=20
Michael Adam
SerNet GmbH, Bahnhofsallee 1b, 37081 G=F6ttingen
phone: +49-551-370000-0, fax: +49-551-370000-9
AG G=F6ttingen, HRB 2816, GF: Dr. Johannes Loxen
http://www.SerNet.DE, mailto: Info @ SerNet.DE

--AKkMM/tm2Mk6Yn/s
Content-Type: application/pgp-signature
Content-Disposition: inline

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.2 (GNU/Linux)
Comment: comment

iD8DBQFIJLityU9JOBhPkDQRAjZeAKCZDAx4yLQ6b4OGGWBzhT HDCOfYdACfaeWK
sqDzqdMK1uI9KAiU2mZDDVA=
=narz
-----END PGP SIGNATURE-----

--AKkMM/tm2Mk6Yn/s--