Tuesday, November 22, 2005, 1:51:05 PM, Robert Shearman wrote:
ChangeLog: Implement directory object in wineserver.
I like the design, but I have a few comments on the patch.
Thank you. That's like 6st revision of it.
+#define DIRECTORY_QUERY (0x0001) +#define DIRECTORY_TRAVERSE (0x0002) +#define DIRECTORY_CREATE_OBJECT (0x0004) +#define DIRECTORY_CREATE_SUBDIRECTORY (0x0008) +#define DIRECTORY_ALL_ACCESS (STANDARD_RIGHTS_REQUIRED | 0xF)
Should these go into winternl.h?
Well they are defined only in ddk/winddk.h in mingw (I don't have SDK or DDK). But probably you are right, they should go into winternl.h
- LIST_FOR_EACH( p, list )
- {
const struct object_name *ptr = LIST_ENTRY( p, const struct object_name, entry );You can replace LIST_FOR_EACH and LIST_ENTRY with LIST_FOR_EACH_ENTRY.
That was direct cut & paste from object.c I decided to keep it the same as well.
- 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.
+/* 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. I didn't want to create one extra variable for each permanent object. As this is a compromise to what windows does. I tried to keep it as flexible as it can be.
+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.
extern int dump_strW( const WCHAR *str, size_t len, FILE *f, const char escape[2] ); +extern void free_unicode_str( struct unicode_str *str ); +extern void dup_unicode_str( const struct unicode_str *src, struct unicode_str *dst );
You could make these both be inline functions.
Ok will do.