[PATCH] more srvsvc stuff

[Stefan Metzmacher]

tridge at samba.org wrote:

> Metze,
> 
>  > here's my latest srvsvc stuff...
> 
> Thanks! I've applied your IDL changes.
> 
> I'm not so sure about the torture changes. I can see why you wanted to
> split up the SRVSVC tests into sub-tests, but I think that perhaps
> that might eventually lead to rather too many tests if we did the same
> thing across all areas of the protocol. What do you think about adding
> a argument list to tests? For example:
> 
>   smbtorture ncacn_np:server RPC-SRVSVC FILES
> 
> then torture.c would notice the additional arguments and pass it if
> present, so each tests would receive a argv[] type array containing
> the additional arguments (if any). That would be useful for other
> tests as well, and allows us to isolate knowledge of individual tests
> a little. In torture/rpc/srvsvc.c you could just apply fnmatch() to
> work out what sub-tests to run. It would mean a fairly large patch to
> add an argv to all the existing tests, but would give us a better
> basis for expanding smbtorture I think.
> 
> On a related matter, I'd love to see torture.c split into pieces, with
> the core SMB tests placed in torture/smb/*.c, torture/rap/*.c etc and
> torture.c itself made into just a small harness program. I think that
> torture.c is rather horrible at the moment. I don't necessarily mean
> that you should do this yourself (although if you have the time that
> would be great!). If anyone else reading this feels like tackling this
> then please send a (well tested) patch.
sounds all good...
maybe can can write a patch...
> 
> I wonder what server you tested against for the CharDev tests? I tried
> them against nt351, nt4, w2k and w2k3 and all gave WERR_NOT_SUPPORTED
> for all calls. That doesn't make the calls very useful :-)
:-)
my winxp box also returns WERR_NOT_SUPPORTED
but as we want a completed api, I want to add tests for all calls

And I think it still makes sense because we can see what we have to
return with our own rpcserver code to match windows exactly...
> 
> I also haven't applied your rpc_server/srvsvc/ patch just yet as I'd
> prefer to not populate our rpc server code with "dummy" type
> returns. That was done a fair bit in the Samba3 rpc server and the
> result was that the dummy code just stayed there for years. Instead
> I'd prefer to either fill in values that make sense for Samba servers
> (for example, a single "C:\" value for the disk enum), or just return
> an appropriate error code. Returning dummyXXX doesn't really help us
> towards having a useful rpc server (even though it does allow us to
> test the infrastructure).
yep, makes sense

maybe I'll add the reall stuff soon
> 
> In your srvsvc.idl, I wonder if perhaps some of the dummy values could
> be collapsed a little? For example, stuff like this:
> 
>         typedef struct{
>                 uint32 dummy;
>         } srvsvc_NetSrvInfo599;
> 
>         typedef struct{
>                 uint32 dummy;
>         } srvsvc_NetSrvInfo1005;
> 
> Could instead be done like this:
> 
>                 [case(599)]    uint32 *dummy;
>                 [case(1005)]   uint32 *dummy;
> 
> The large number of dummy structures just makes it harder to read
> through the IDL (due to sheer volume) and doesn't really add much.
> 
> We should probably also add support for ranges in case(), like this:
> 
>                 [case(1509-1516)]  uint32 *dummy;
> 
> From chapter 4 of the opengroup spec, it looks like a case can contain
> a comma separated list of case values, although pidl doesn't accept
> that yet. We should probably add that, then also extend it to allow
> for ranges, either using 10-20 syntax or 10..20 syntax. That would
> allow things like NetSrvInfo to be described in a very compact manner,
> making it both more readable and easier to maintain.
> 
> What do you think?

sounds very good


-- 

metze

-------------------------------------------
Stefan (metze) Metzmacher <metze at metzemix.de>