Implement directory object in wineserver.
Robert Shearman
rob at codeweavers.com
Tue Nov 22 16:14:15 CST 2005
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
More information about the wine-devel
mailing list