Vitaliy Margolen wrote:
+ struct object *obj, *parent;
+ struct unicode_str name_l = *attr->name;
What does name_l stand for?
L for local. I don't like to give local variables to long of the names.
I'd drop the "_l" suffix. It confused me when reading your patch into
thinking it was some kind of special name, whereas it was just the name.
+/* Name space initialization */
+
+struct object *permanent_obj[25];
+int permanent_obj_cnt = 0;
This looks a bit ugly to me. Why not just keep track of the individual
objects that need to be kept around in named variables?
Because there will be more. Potentially we might have device + symlink
for stuff like named pipes, mailslots, serial & paralel ports, etc.
Yes, and these should be created at startup.
I didn't want to create one extra variable for each permanent object.
There should be less than ten objects altogether, so why not? It doesn't
have a level of complexity that the "permanent object" concept does.
As
this is a compromise to what windows does. I tried to keep it as flexible
as it can be.
Granted, but overdesign is as bad as not being flexible enough. Is there
a need for this array of permanent objects outside of this patch?
+void dup_unicode_str( const struct unicode_str *src, struct unicode_str *dst )
+{
+ if (!src || !dst) return;
I think it would probably be better to let the function crash on both of
these conditions. If the client doesn't check if src is null then dst
when be left unintialized and cause a crash later.
Mm no. It is really not good to crash server because of invalid
parameters. The thing is, that I'm using NULL pointers when calling this
function, so I've added extra checks here to not crash. This is internal
to server function, so I don't really see a problem hawing an extra
check.
I'm not talking about crashing because of bad input from an application,
but bad input from a developer. The only use of this function is already
guarded against using NULL pointers. This is an internal to server
function, so I don't really see a problem in crashing if a future
developer isn't careful about what he/she passes to it. Even a good
developer might not consider that dup_unicode_str would leave dst
uninitialized when a NULL src is passed in. If you make them do the
check themselves, then they likely will think about it.
--
Rob Shearman
