[PATCH] more srvsvc stuff

tridge at samba.org tridge at samba.org
Sat Dec 20 22:24:08 GMT 2003


 > 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.

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 :-)

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).

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?

Cheers, Tridge

PS: I'm away over christmas, so expect slow replies :)

More information about the samba-technical mailing list