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.