Vitaliy Margolen wrote:
> ChangeLog:
> Implement directory object in wineserver.
I like the design, but I have a few comments on the patch.
> server/Makefile.in | 1 server/directory.c | 384
> +++++++++++++++++++++++++++++++++++++++++++++++++++
> server/main.c | 1 server/object.c | 64 +++++++--
> server/object.h | 36 +++++
> server/protocol.def | 21 +++
> server/request.c | 1 server/request.h | 11 +
> server/unicode.c | 14 ++
> server/unicode.h | 2 10 files changed, 521 insertions(+), 14
> deletions(-)
> create mode 100644 server/directory.c
>
>------------------------------------------------------------------------
>
>760fd4b8fd36abf1b281bfa2f963bf499bc35be5
>diff --git a/server/Makefile.in b/server/Makefile.in
>index 47687c0..50a5049 100644
>--- a/server/Makefile.in
>+++ b/server/Makefile.in
>@@ -17,6 +17,7 @@ C_SRCS = \
> context_sparc.c \
> context_x86_64.c \
> debugger.c \
>+ directory.c \
> event.c \
> fd.c \
> file.c \
>diff --git a/server/directory.c b/server/directory.c
>new file mode 100644
>index 0000000..d8f168d
>--- /dev/null
>+++ b/server/directory.c
>@@ -0,0 +1,384 @@
>+/*
>+ * Server-side directory object management
>+ *
>+ * Copyright (C) 2005 Vitaliy Margolen
>+ *
>+ * This library is free software; you can redistribute it and/or
>+ * modify it under the terms of the GNU Lesser General Public
>+ * License as published by the Free Software Foundation; either
>+ * version 2.1 of the License, or (at your option) any later version.
>+ *
>+ * This library is distributed in the hope that it will be useful,
>+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
>+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
>+ * Lesser General Public License for more details.
>+ *
>+ * You should have received a copy of the GNU Lesser General Public
>+ * License along with this library; if not, write to the Free Software
>+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
>+ *
>+ */
>+
>+#include "config.h"
>+#include "wine/port.h"
>+
>+#include <assert.h>
>+#include <stdarg.h>
>+#include <stdlib.h>
>+#include <sys/types.h>
>+
>+#include "windef.h"
>+#include "winternl.h"
>+
>+#include "handle.h"
>+#include "request.h"
>+#include "process.h"
>+#include "unicode.h"
>+#include "wine/list.h"
>+#include "object.h"
>+
>+#define HASH_SIZE 37
>+
>+#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?
>+
>+/* Global name space */
>+struct directory *root_directory;
>+
>+struct directory
>+{
>+ struct object obj; /* object header */
>+ struct list entries[HASH_SIZE]; /* array of hash entry lists */
>+};
>+
>+static void directory_dump( struct object *obj, int verbose );
>+static struct object *directory_lookup_name( struct object *obj, struct unicode_str *name,
>+ unsigned int attr );
>+
>+static const struct object_ops directory_ops =
>+{
>+ sizeof(struct directory), /* size */
>+ directory_dump, /* dump */
>+ no_add_queue, /* add_queue */
>+ NULL, /* remove_queue */
>+ NULL, /* signaled */
>+ NULL, /* satisfied */
>+ no_signal, /* signal */
>+ no_get_fd, /* get_fd */
>+ directory_lookup_name, /* lookup_name */
>+ no_close_handle, /* close_handle */
>+ no_destroy /* destroy */
>+};
>+
>+static void directory_dump( struct object *obj, int verbose )
>+{
>+ struct directory *dir = (struct directory *)obj;
>+ assert( obj->ops == &directory_ops );
>+
>+ fputs( "Directory ", stderr );
>+ dump_object_name( obj );
>+ if (verbose)
>+ {
>+ unsigned int i;
>+
>+ fputs( " entries:\n", stderr );
>+ for (i = 0; i < HASH_SIZE; i++)
>+ {
>+ struct list *p = &dir->entries[i];
>+ struct object_name *ptr;
>+
>+ LIST_FOR_EACH_ENTRY(ptr, p, struct object_name, entry)
>+ {
>+ fputs( " ", stderr );
>+ dump_object_name( ptr->obj );
>+ fputc( '\n', stderr );
>+ }
>+ }
>+ }
>+ else
>+ fputc( '\n', stderr );
>+}
>+
>+/* find an object by its name in a given directory; the refcount is incremented
>+ * name - is a relative name */
>+static struct object *find_object_in_dir( struct directory *dir, const WCHAR *name,
>+ size_t len, unsigned int attr )
>+{
>+ const struct list *list, *p;
>+
>+ if (!name || !len) return NULL;
>+
>+ list = &dir->entries[ get_name_hash( name, len ) % HASH_SIZE ];
>+ 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.
>+ if (!ptr || ptr->len != len) continue;
>+ if (attr & OBJ_CASE_INSENSITIVE)
>+ {
>+ if (!strncmpiW( ptr->name, name, len/sizeof(WCHAR) )) return grab_object( ptr->obj );
>+ }
>+ else
>+ {
>+ if (!memcmp( ptr->name, name, len )) return grab_object( ptr->obj );
>+ }
>+ }
>+ return NULL;
>+}
>+
>+static struct object *directory_lookup_name( struct object *obj, struct unicode_str *name,
>+ unsigned int attr )
>+{
>+ struct object *found;
>+ WCHAR *p;
>+ size_t sz;
>+ assert( obj->ops == &directory_ops );
>+
>+ if (!(p = memchrW( name->str, '\\', name->len / sizeof(WCHAR) )))
>+ /* Last element in the path name */
>+ sz = name->len;
>+ else
>+ sz = (p - name->str) * sizeof(WCHAR);
>+
>+ if ((found = find_object_in_dir( (struct directory *)obj, name->str, sz, attr )))
>+ {
>+ /* Skip trailing \\ */
>+ if (p)
>+ {
>+ p++;
>+ sz += sizeof(WCHAR);
>+ }
>+ /* Move to the next element*/
>+ name->str = p;
>+ name->len -= sz;
>+ return found;
>+ }
>+
>+ if (name->str)
>+ {
>+ if (sz == 0) /* Double backslash */
>+ set_error( STATUS_OBJECT_NAME_INVALID );
>+ else if (p) /* Path still has backslashes */
>+ set_error( STATUS_OBJECT_PATH_NOT_FOUND );
>+ }
>+ return NULL;
>+}
>+
>+/******************************************************************************/
>+
>+/* Create root directory - it's a special case when no namespace exists yet.*/
>+struct directory *create_root( void )
>+{
>+ static const WCHAR rootW[]={'\\'};
>+ static const struct unicode_str root = {rootW, sizeof(rootW)};
>+ struct directory *dir;
>+ struct object_name *name_ptr;
>+
>+ if (!(name_ptr = alloc_name( &root ))) return NULL;
>+ if ((dir = alloc_object( &directory_ops )))
>+ {
>+ unsigned int i;
>+ /* root has no parents */
>+ list_init( &name_ptr->entry );
>+ name_ptr->obj = &dir->obj;
>+ dir->obj.name = name_ptr;
>+ for (i = 0; i < HASH_SIZE; i++)
>+ list_init( &dir->entries[i] );
>+ }
>+ else free( name_ptr );
>+ return dir;
>+}
>+
>+struct directory *create_directory( const struct object_attr *attr )
>+{
>+ struct directory *dir;
>+
>+ if ((dir = create_named_object_dir( attr, &directory_ops )))
>+ {
>+ if (get_error() != STATUS_OBJECT_NAME_EXISTS)
>+ {
>+ unsigned int i;
>+ for (i = 0; i < HASH_SIZE; i++)
>+ list_init( &dir->entries[i] );
>+ }
>+ }
>+ return dir;
>+}
>+
>+static void insert_into_dir( struct directory* dir, struct object *obj )
>+{
>+ int hash = get_name_hash( obj->name->name, obj->name->len) % HASH_SIZE;
>
>
Missing space after obj->name->len.
>+
>+ grab_object( dir );
>+ obj->name->parent = &dir->obj;
>+ list_add_head( &dir->entries[hash], &obj->name->entry );
>+}
>+
>+/******************************************************************************
>+ * Find an object by it's name in a given root object
>+ *
>+ * PARAMS
>+ * attr [I] rootdir, name and attributes
>+ * name_left [O] [optional] leftover name if object is not found
>+ *
>+ * RETURNS
>+ * NULL: If params are invalid
>+ * Found: If object with exact name is found returns that object. (name_left->len == 0)
>+ * Object's refcount is incremented
>+ * Not found: The last matched parent. (name_left->len > 0)
>+ * Parent's refcount is incremented.
>+ */
>+struct object *find_object_dir( const struct object_attr *attr, struct unicode_str *name_left )
>+{
>+ struct object *obj, *parent;
>+ struct unicode_str name_l = *attr->name;
>
>
What does name_l stand for?
>+
>+ /* Arguments check:
>+ * - Either name or root have to be specified
>+ * - If root is specified path shouldn't start with backslash */
>+ if (!attr ||
>+ (!attr->rootdir && (!name_l.str || !name_l.len)) ||
>+ (name_l.len && ((*name_l.str == '\\' && attr->rootdir) ||
>+ (*name_l.str != '\\' && !attr->rootdir))))
>+ {
>+ set_error( STATUS_OBJECT_PATH_SYNTAX_BAD );
>+ return NULL;
>+ }
>+
>+ if (attr->rootdir)
>+ parent = get_handle_obj( current->process, attr->rootdir, 0, &directory_ops );
>+ else
>+ parent = grab_object( root_directory );
>+ if (!parent) return NULL;
>+
>+ /* Skip leading backslash */
>+ if (name_l.len && *name_l.str == '\\')
>+ {
>+ name_l.str++;
>+ name_l.len -= sizeof(WCHAR);
>+ }
>+
>+ /* Special case for opening RootDirectory */
>+ if (!name_l.len) goto done;
>+
>+ while ((obj = parent->ops->lookup_name( parent, &name_l, attr->attributes )))
>+ {
>+ /* move to the next element */
>+ release_object ( parent );
>+ parent = obj;
>+ }
>+ if (get_error())
>+ {
>+ release_object ( parent );
>+ return NULL;
>+ }
>+
>+ done:
>+ if (name_left) dup_unicode_str(&name_l, name_left);
>+ return parent;
>+}
>+
>
>
>@@ -311,3 +309,41 @@ int no_close_handle( struct object *obj,
> void no_destroy( struct object *obj )
> {
> }
>+
>+/* 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?
>+
>+static inline void create_default_dir( const WCHAR *n, size_t nl)
>+{
>+ struct unicode_str name;
>+ struct object_attr attr = {NULL, &name, 0};
>+ struct directory *dir;
>+ name.str = n;
>+ name.len = nl;
>+ if ((dir = create_directory( &attr ) ))
>+ permanent_obj[permanent_obj_cnt++] = (struct object *)dir;
>+}
>+
>+static void init_namespace_dirs(void)
>+{
>+ /* Directories */
>+ static const WCHAR dir1W[]={'\\','?','?'};
>+ static const WCHAR dir2W[]={'\\','B','a','s','e','N','a','m','e','d','O','b','j','e','c','t','s'};
>+
>+ root_directory = create_root();
>+ permanent_obj[permanent_obj_cnt++] = (struct object *)root_directory;
>+ create_default_dir( dir1W, sizeof(dir1W) );
>+ create_default_dir( dir2W, sizeof(dir2W) );
>+}
>+
>+void init_namespace( void )
>+{
>+ init_namespace_dirs();
>+}
>+
>+void close_namespace( void )
>+{
>+ while(permanent_obj_cnt) release_object( permanent_obj[--permanent_obj_cnt] );
>+}
>
>
>diff --git a/server/unicode.c b/server/unicode.c
>index 16aea3d..c275f86 100644
>--- a/server/unicode.c
>+++ b/server/unicode.c
>@@ -68,3 +68,17 @@ int dump_strW( const WCHAR *str, size_t
> count += pos - buffer;
> return count;
> }
>+
>+void inline free_unicode_str( struct unicode_str *str )
>+{
>+ if (str->str) free( (WCHAR*)str->str );
>+ str->str = NULL;
>+ str->len = 0;
>+}
>+
>+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.
>+ dst->str = memdup( src->str, src->len );
>+ dst->len = src->len;
>+}
>diff --git a/server/unicode.h b/server/unicode.h
>index f745705..ebd1343 100644
>--- a/server/unicode.h
>+++ b/server/unicode.h
>@@ -34,5 +34,7 @@ static inline WCHAR *strdupW( const WCHA
> }
>
> 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.
--
Rob Shearman
