Discussion:
[PATCHS]: parsing get_dfs_referral messages
(too old to reply)
Matthieu Patou
2009-12-10 12:40:17 UTC
Permalink
Hello,

Please in my repo: http://repo.or.cz/w/Samba/ekacnet.git branch msdfs
from HEAD to 5ca793e a bunch of patch for parsing get_dfs_referral
message.

* 0001-pidl-Introduce-new-dirrective-relative_short.patch, this patch
introduce the relative_short directive which is the same as the relative
on but with an offset stored in 2 bytes. It's needed because DFS_REFERAL
messages have such kind of offset (
http://msdn.microsoft.com/en-us/library/cc227017%28PROT.10%29.aspx)

* 0002-pidl-create-trans.idl-IDL-for-TRANSACTIONS2-extensio.patch, this
is the IDL for the TRANSACATIONS2 extension which support the call
get_dfs_referral
* 0003-torture-torture-tests-for-get_dfs_referral-of-trans..patch, a
small torture test to check that we are able to pull from a known
get_dfs_referral message and also that we are able to push the result to
get the identical blob
* 0004-idl-add-generated-code.patch


Let me know.

Matthieu.
Matthias Dieter Wallnöfer
2009-12-10 15:29:58 UTC
Permalink
Hi ekacnet,

nice work! In general do you have all newer patches written by you in
your repo? As you know this simplifies the review and merge procedure much.

A first suggestion: please be a bit careful with code intendation, e.g.
this we don't like that much (in C we generally prefer tabs, in python
spaces but also tabs - but we try to don't mix them in the same file).
+ typedef struct {
+ uint16 size;
+ uint16 server_type;
+ uint16 entry_flags;
+ nstring *share_name;
+ } trans2_dfs_referral_v1;
It's useful to consider always first the commit DIFFs to fix those
"cosmetics" or remove unwanted cruft (using "gitweb", "gitk", "git show"
or "git diff") before publishing here or on IRC.

For the content I would invite someone skilled with the s4 file server
to answer.

Matthias
Hello,
Please in my repo: http://repo.or.cz/w/Samba/ekacnet.git branch msdfs
from HEAD to 5ca793e a bunch of patch for parsing get_dfs_referral
message.
* 0001-pidl-Introduce-new-dirrective-relative_short.patch, this patch
introduce the relative_short directive which is the same as the
relative on but with an offset stored in 2 bytes. It's needed because
DFS_REFERAL messages have such kind of offset (
http://msdn.microsoft.com/en-us/library/cc227017%28PROT.10%29.aspx)
* 0002-pidl-create-trans.idl-IDL-for-TRANSACTIONS2-extensio.patch,
this is the IDL for the TRANSACATIONS2 extension which support the
call get_dfs_referral
* 0003-torture-torture-tests-for-get_dfs_referral-of-trans..patch, a
small torture test to check that we are able to pull from a known
get_dfs_referral message and also that we are able to push the result
to get the identical blob
* 0004-idl-add-generated-code.patch
Let me know.
Matthieu.
Matthieu Patou
2009-12-10 17:10:47 UTC
Permalink
Hi Matthias,
Post by Matthias Dieter Wallnöfer
Hi ekacnet,
nice work! In general do you have all newer patches written by you in
your repo? As you know this simplifies the review and merge procedure much.
A first suggestion: please be a bit careful with code intendation,
e.g. this we don't like that much (in C we generally prefer tabs, in
python spaces but also tabs - but we try to don't mix them in the same
file).
+ typedef struct {
+ uint16 size;
+ uint16 server_type;
+ uint16 entry_flags;
+ nstring *share_name;
+ } trans2_dfs_referral_v1;
It's useful to consider always first the commit DIFFs to fix those
"cosmetics" or remove unwanted cruft (using "gitweb", "gitk", "git
show" or "git diff") before publishing here or on IRC.
Thanks for pointing this matthias ! I already changed this in the repo.
In fact I'm not hopping to get this patch right from the first shot but
more to get information in what is not correct.

Matthieu.
Post by Matthias Dieter Wallnöfer
For the content I would invite someone skilled with the s4 file server
to answer.
Matthias
Hello,
Please in my repo: http://repo.or.cz/w/Samba/ekacnet.git branch msdfs
from HEAD to 5ca793e a bunch of patch for parsing get_dfs_referral
message.
* 0001-pidl-Introduce-new-dirrective-relative_short.patch, this patch
introduce the relative_short directive which is the same as the
relative on but with an offset stored in 2 bytes. It's needed because
DFS_REFERAL messages have such kind of offset (
http://msdn.microsoft.com/en-us/library/cc227017%28PROT.10%29.aspx)
* 0002-pidl-create-trans.idl-IDL-for-TRANSACTIONS2-extensio.patch,
this is the IDL for the TRANSACATIONS2 extension which support the
call get_dfs_referral
* 0003-torture-torture-tests-for-get_dfs_referral-of-trans..patch, a
small torture test to check that we are able to pull from a known
get_dfs_referral message and also that we are able to push the result
to get the identical blob
* 0004-idl-add-generated-code.patch
Let me know.
Matthieu.
Matthieu Patou
2009-12-10 12:31:14 UTC
Permalink
Hello,

Please find attach a bunch of patch for parsing get_dfs_referral message.

* 0001-pidl-Introduce-new-dirrective-relative_short.patch, this patch
introduce the relative_short directive which is the same as the relative
on but with an offset stored in 2 bytes. It's needed because DFS_REFERAL
messages have such kind of offset (
http://msdn.microsoft.com/en-us/library/cc227017%28PROT.10%29.aspx)

* 0002-pidl-create-trans.idl-IDL-for-TRANSACTIONS2-extensio.patch, this
is the IDL for the TRANSACATIONS2 extension which support the call
get_dfs_referral
* 0003-torture-torture-tests-for-get_dfs_referral-of-trans..patch, a
small torture test to check that we are able to pull from a known
get_dfs_referral message and also that we are able to push the result to
get the identical blob
* 0004-idl-add-generated-code.patch


Let me know.

Matthieu.
Stefan (metze) Metzmacher
2009-12-12 09:09:25 UTC
Permalink
Post by Matthieu Patou
Hello,
Please find attach a bunch of patch for parsing get_dfs_referral message.
* 0001-pidl-Introduce-new-dirrective-relative_short.patch, this patch
introduce the relative_short directive which is the same as the relative
on but with an offset stored in 2 bytes. It's needed because DFS_REFERAL
messages have such kind of offset (
http://msdn.microsoft.com/en-us/library/cc227017%28PROT.10%29.aspx)
I'll take this splited into a librpc/ndr and a pidl/ patch.
Post by Matthieu Patou
* 0002-pidl-create-trans.idl-IDL-for-TRANSACTIONS2-extensio.patch, this
is the IDL for the TRANSACATIONS2 extension which support the call
get_dfs_referral
I think we should have a dfsblobs.idl similar to drsblobs.idl
instead of having a trans.idl.

Types like TRANS_FLAGS_REFERRAL are confusing, it has nothing directly
todo with (smb_)trans. The prefix should be DFS_ or something similar.

For SMB2 this blobs are transferred via Ioctl calls.
Post by Matthieu Patou
* 0003-torture-torture-tests-for-get_dfs_referral-of-trans..patch, a
small torture test to check that we are able to pull from a known
get_dfs_referral message and also that we are able to push the result to
get the identical blob
You should add the test under torture/ndr/ where similar tests already
exist instead of creating a complete new subsystem.
Post by Matthieu Patou
* 0004-idl-add-generated-code.patch
As source3 doesn't use this (yet), we should not add generated files.
Instead update .gitignore.

metze

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 260 bytes
Desc: OpenPGP digital signature
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20091212/187bb16a/attachment.pgp>
Matthieu Patou
2009-12-13 15:57:23 UTC
Permalink
Hello stephan,

Find attached updates patches due to your comments:
*0001-pidl-Introduce-new-directive-relative_short.patch
*0002-ndr-add-supporting-C-functions-for-directive-relativ.patch
Split of the pidl changes

*0003-idl-introduce-new-file-dfsblobs.idl.patch
Renaming of the trans.idl to have something like drsblobs.idl. The
prefix has been changed also

*0004-torture-add-new-tests-for-dfsblobs.patch
Torture tests integrated into ndr tests

*0005-add-ignore-for-autogenrated-files-as-they-are-not-us.patch
Ignore for the autogenerated files.

It's also available at

http://repo.or.cz/w/Samba/ekacnet.git/shortlog/refs/heads/msdfs-review

Let me know.

Matthieu.
Post by Stefan (metze) Metzmacher
Post by Matthieu Patou
Hello,
Please find attach a bunch of patch for parsing get_dfs_referral message.
* 0001-pidl-Introduce-new-dirrective-relative_short.patch, this patch
introduce the relative_short directive which is the same as the relative
on but with an offset stored in 2 bytes. It's needed because DFS_REFERAL
messages have such kind of offset (
http://msdn.microsoft.com/en-us/library/cc227017%28PROT.10%29.aspx)
I'll take this splited into a librpc/ndr and a pidl/ patch.
Post by Matthieu Patou
* 0002-pidl-create-trans.idl-IDL-for-TRANSACTIONS2-extensio.patch, this
is the IDL for the TRANSACATIONS2 extension which support the call
get_dfs_referral
I think we should have a dfsblobs.idl similar to drsblobs.idl
instead of having a trans.idl.
Types like TRANS_FLAGS_REFERRAL are confusing, it has nothing directly
todo with (smb_)trans. The prefix should be DFS_ or something similar.
For SMB2 this blobs are transferred via Ioctl calls.
Post by Matthieu Patou
* 0003-torture-torture-tests-for-get_dfs_referral-of-trans..patch, a
small torture test to check that we are able to pull from a known
get_dfs_referral message and also that we are able to push the result to
get the identical blob
You should add the test under torture/ndr/ where similar tests already
exist instead of creating a complete new subsystem.
Post by Matthieu Patou
* 0004-idl-add-generated-code.patch
As source3 doesn't use this (yet), we should not add generated files.
Instead update .gitignore.
metze
Continue reading on narkive:
Loading...