[PATCH] Use refcount_t for peropen reference counting in libnetfs.

[Flavio Cruz]

* libnetfs/netfs.h: Use refcount_t.
* libnetfs/make-peropen.c: Initialize to 1 with refcount_init just like in 
libdiskfs.
* libnetfs/make-protid.c: Don't increment the count here. Do it like libdiskfs.
* libnetfs/io-duplicate.c: Add refcount_ref since netfs_make_protid no longer 
increments the refcount.
* libnetfs/io-reauthenticate.c: Likewise.
* libnetfs/io-restrict-auth.c: Likewise.
* libnetfs/release-peropen.c: Dereference without locking.
---
 libnetfs/io-duplicate.c  |  1 +
 libnetfs/io-reauthenticate.c |  2 ++
 libnetfs/io-restrict-auth.c  |  2 ++
 libnetfs/make-peropen.c  |  2 +-
 libnetfs/make-protid.c   |  1 -
 libnetfs/netfs.h |  2 +-
 libnetfs/release-peropen.c   | 38 ++
 7 files changed, 25 insertions(+), 23 deletions(-)

diff --git a/libnetfs/io-duplicate.c b/libnetfs/io-duplicate.c
index b2c3a3a..263f9e8 100644
--- a/libnetfs/io-duplicate.c
+++ b/libnetfs/io-duplicate.c
@@ -34,6 +34,7 @@ netfs_S_io_duplicate (struct protid *user,
   if (err)
 return err;
   
+  refcount_ref (>po->refcnt);
   pthread_mutex_lock (>po->np->lock);
   newpi = netfs_make_protid (user->po, clone);
   *newport = ports_get_right (newpi);
diff --git a/libnetfs/io-reauthenticate.c b/libnetfs/io-reauthenticate.c
index 8ff4182..b2d4a44 100644
--- a/libnetfs/io-reauthenticate.c
+++ b/libnetfs/io-reauthenticate.c
@@ -34,12 +34,14 @@ netfs_S_io_reauthenticate (struct protid *user, mach_port_t 
rend_port)
   /* This routine must carefully ignore EINTR because we
  are a simpleroutine, so callers won't know to restart. */
 
+  refcount_ref (>po->refcnt);
   pthread_mutex_lock (>po->np->lock);
   do
 newpi = netfs_make_protid (user->po, 0);
   while (! newpi && errno == EINTR);
   if (! newpi)
 {
+  refcount_deref (>po->refcnt);
   pthread_mutex_unlock (>po->np->lock);
   return errno;
 }
diff --git a/libnetfs/io-restrict-auth.c b/libnetfs/io-restrict-auth.c
index 0c3403d..79b7d09 100644
--- a/libnetfs/io-restrict-auth.c
+++ b/libnetfs/io-restrict-auth.c
@@ -43,6 +43,7 @@ netfs_S_io_restrict_auth (struct protid *user,
 return err;
 
   pthread_mutex_lock (>po->np->lock);
+  refcount_ref (>po->refcnt);
   newpi = netfs_make_protid (user->po, new_user);
   if (newpi)
 {
@@ -52,6 +53,7 @@ netfs_S_io_restrict_auth (struct protid *user,
 }
   else
 {
+  refcount_deref (>po->refcnt);
   pthread_mutex_unlock (>po->np->lock);
   iohelp_free_iouser (new_user);
   err = ENOMEM;
diff --git a/libnetfs/make-peropen.c b/libnetfs/make-peropen.c
index f7be58b..413e914 100644
--- a/libnetfs/make-peropen.c
+++ b/libnetfs/make-peropen.c
@@ -31,7 +31,7 @@ netfs_make_peropen (struct node *np, int flags, struct 
peropen *context)
 
   po->filepointer = 0;
   po->lock_status = LOCK_UN;
-  po->refcnt = 0;
+  refcount_init (>refcnt, 1);
   po->openstat = flags;
   po->np = np;
   po->path = NULL;
diff --git a/libnetfs/make-protid.c b/libnetfs/make-protid.c
index bf18283..995ac1e 100644
--- a/libnetfs/make-protid.c
+++ b/libnetfs/make-protid.c
@@ -36,7 +36,6 @@ netfs_make_protid (struct peropen *po, struct iouser *cred)
   if (errno)
 return 0;
 
-  po->refcnt++;
   pi->po = po;
   pi->user = cred;
   pi->shared_object = MACH_PORT_NULL;
diff --git a/libnetfs/netfs.h b/libnetfs/netfs.h
index fbe2c60..3f94ccd 100644
--- a/libnetfs/netfs.h
+++ b/libnetfs/netfs.h
@@ -51,7 +51,7 @@ struct peropen
 {
   loff_t filepointer;
   int lock_status;
-  int refcnt;
+  refcount_t refcnt;
   int openstat;
 
   struct node *np;
diff --git a/libnetfs/release-peropen.c b/libnetfs/release-peropen.c
index c206b43..01af97d 100644
--- a/libnetfs/release-peropen.c
+++ b/libnetfs/release-peropen.c
@@ -24,29 +24,27 @@
 void
 netfs_release_peropen (struct peropen *po)
 {
+  if (refcount_deref (>refcnt) > 0)
+return;
+
   pthread_mutex_lock (>np->lock);
-  if (--po->refcnt)
-pthread_mutex_unlock (>np->lock);
-  else
-{
-  if (po->root_parent)
-   mach_port_deallocate (mach_task_self (), po->root_parent);
+  if (po->root_parent)
+mach_port_deallocate (mach_task_self (), po->root_parent);
 
-  if (po->shadow_root && po->shadow_root != po->np)
-   {
- pthread_mutex_lock (>shadow_root->lock);
- netfs_nput (po->shadow_root);
-   }
-  if (po->shadow_root_parent)
-   mach_port_deallocate (mach_task_self (), po->shadow_root_parent);
+  if (po->shadow_root && po->shadow_root != po->np)
+{
+  pthread_mutex_lock (>shadow_root->lock);
+  netfs_nput (po->shadow_root);
+}
+  if (po->shadow_root_parent)
+mach_port_deallocate (mach_task_self (), po->shadow_root_parent);
 
-  if (po->lock_status != LOCK_UN)
-   fshelp_acquire_lock (>np->userlock, >lock_status,
->np->lock, LOCK_UN);
+  if (po->lock_status != LOCK_UN)
+fshelp_acquire_lock (>np->userlock, >lock_status,
+>np->lock, LOCK_UN);
 
-  netfs_nput (po->np);
+  

[PATCH] Remove libfshelp/trans.h and libfshelp/locks.h.

[Flavio Cruz]

* libfshelp/locks.h: Remove.
* libfshelp/trans.h: Remove. struct transboot is not used anywhere.
* libfshelp/fetch-root.c: Adjust includes.
* libfshelp/lock-acquire.c: Likewise.
* libfshelp/lock-init.c: Likewise.
---
 libfshelp/fetch-root.c   |  9 ++---
 libfshelp/lock-acquire.c |  5 -
 libfshelp/lock-init.c|  4 +++-
 libfshelp/locks.h| 28 
 libfshelp/trans.h| 32 
 5 files changed, 13 insertions(+), 65 deletions(-)
 delete mode 100644 libfshelp/locks.h
 delete mode 100644 libfshelp/trans.h

diff --git a/libfshelp/fetch-root.c b/libfshelp/fetch-root.c
index 712c11f..1b6739e 100644
--- a/libfshelp/fetch-root.c
+++ b/libfshelp/fetch-root.c
@@ -18,11 +18,14 @@
along with this program; if not, write to the Free Software
Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA. */
 
-#include "trans.h"
-#include 
 #include 
-#include 
 #include 
+#include 
+#include 
+#include 
+#include 
+
+#include "fshelp.h"
 
 error_t
 fshelp_fetch_root (struct transbox *box, void *cookie,
diff --git a/libfshelp/lock-acquire.c b/libfshelp/lock-acquire.c
index f68c30a..07df428 100644
--- a/libfshelp/lock-acquire.c
+++ b/libfshelp/lock-acquire.c
@@ -19,7 +19,10 @@
 
 /* Written by Michael I. Bushnell.  */
 
-#include "locks.h"
+#include 
+#include 
+
+#include "fshelp.h"
 
 #define EWOULDBLOCK EAGAIN /* XXX */
 
diff --git a/libfshelp/lock-init.c b/libfshelp/lock-init.c
index 21fb0d7..bb1f1f8 100644
--- a/libfshelp/lock-init.c
+++ b/libfshelp/lock-init.c
@@ -19,7 +19,9 @@
 
 /* Written by Michael I. Bushnell.  */
 
-#include "locks.h"
+#include 
+
+#include "fshelp.h"
 
 /* Initialize a lock box. */
 void
diff --git a/libfshelp/locks.h b/libfshelp/locks.h
deleted file mode 100644
index a950f61..000
--- a/libfshelp/locks.h
+++ /dev/null
@@ -1,28 +0,0 @@
-/*
-   Copyright (C) 1994 Free Software Foundation
-
-This file is part of the GNU Hurd.
-
-The GNU Hurd is free software; you can redistribute it and/or modify
-it under the terms of the GNU General Public License as published by
-the Free Software Foundation; either version 2, or (at your option)
-any later version.
-
-The GNU Hurd 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 General Public License for more details.
-
-You should have received a copy of the GNU General Public License
-along with the GNU Hurd; see the file COPYING.  If not, write to
-the Free Software Foundation, 675 Mass Ave, Cambridge, MA 02139, USA.  */
-
-/* Written by Michael I. Bushnell.  */
-
-#include 
-#include 
-#include 
-#include 
-#include "fshelp.h"
-#include 
-#include 
diff --git a/libfshelp/trans.h b/libfshelp/trans.h
deleted file mode 100644
index a9ea648..000
--- a/libfshelp/trans.h
+++ /dev/null
@@ -1,32 +0,0 @@
-/* 
-   Copyright (C) 1994 Free Software Foundation
-
-   This program is free software; you can redistribute it and/or
-   modify it under the terms of the GNU General Public License as
-   published by the Free Software Foundation; either version 2, or (at
-   your option) any later version.
-
-   This program 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
-   General Public License for more details.
-
-   You should have received a copy of the GNU General Public License
-   along with this program; if not, write to the Free Software
-   Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA. */
-
-#include 
-#include 
-#include 
-#include 
-#include "fshelp.h"
-
-struct transboot
-{
-  struct port_info pi;
-  file_t node;
-  struct trans_link *link;
-};
-
-pthread_spinlock_t _fshelp_translistlock;
-struct trans_link *_fshelp_translist;
-- 
2.6.4

[PATCH gnumach 2/3] vm: allocate a large map for all objects larger than SMALL_SIZE

[Justus Winter]

* vm/vm_external.c (vm_external_create): Allocate a large map for all
objects larger than SMALL_SIZE.  'vm_external_state_{g,s}et' can deal
with offsets larger than 'LARGE_SIZE', so currently objects larger
than 'LARGE_SIZE' are missing out on the optimization.
---
 vm/vm_external.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/vm/vm_external.c b/vm/vm_external.c
index 097a9b1..7584a2b 100644
--- a/vm/vm_external.c
+++ b/vm/vm_external.c
@@ -69,7 +69,7 @@ vm_external_t vm_external_create(vm_offset_t size)
result->existence_map =
 (char *) 
kmem_cache_alloc(_object_small_existence_map_cache);
result->existence_size = SMALL_SIZE;
-   } else if (bytes <= LARGE_SIZE) {
+   } else {
result->existence_map =
 (char *) 
kmem_cache_alloc(_object_large_existence_map_cache);
result->existence_size = LARGE_SIZE;
-- 
2.1.4

[PATCH gnumach 3/3] vm: initialize external maps

[Justus Winter]

* vm/vm_external.c (vm_external_create): Initialize allocated maps.
---
 vm/vm_external.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/vm/vm_external.c b/vm/vm_external.c
index 7584a2b..6e71918 100644
--- a/vm/vm_external.c
+++ b/vm/vm_external.c
@@ -74,6 +74,7 @@ vm_external_t vm_external_create(vm_offset_t size)
 (char *) 
kmem_cache_alloc(_object_large_existence_map_cache);
result->existence_size = LARGE_SIZE;
}
+   memset (result->existence_map, 0, result->existence_size);
return(result);
 }
 
-- 
2.1.4

[PATCH gnumach 1/3] vm: remove unused field from struct vm_external

[Justus Winter]

* vm/vm_external.h (struct vm_external): Remove unused field
'existence_count'.
---
 vm/vm_external.h | 5 +
 1 file changed, 5 insertions(+)

diff --git a/vm/vm_external.h b/vm/vm_external.h
index 55c9e48..4e44ddf 100644
--- a/vm/vm_external.h
+++ b/vm/vm_external.h
@@ -46,9 +46,14 @@ typedef struct vm_external {
 * been written to backing
 * storage.
 */
+#if 0
+   /* XXX: Currently, existence_count is not used.  I guess it
+  could be useful to get rid of the map if the count drops to
+  zero.  */
int existence_count;/* Number of bits turned on in
 * existence_map.
 */
+#endif
 } *vm_external_t;
 
 #defineVM_EXTERNAL_NULL((vm_external_t) 0)
-- 
2.1.4

[PATCH] Use libihash to store directory entries in ftpfs.

[Flavio Cruz]

* ftpfs/ftpfs.h: Add dir_locp for each directory entry and replace the table 
with a libihash table.
* ftpfs/dir.c: Modify the code to use libihash. Remove several functions such 
as rehash and insert.
---
 ftpfs/dir.c   | 142 --
 ftpfs/ftpfs.h |  17 +++
 2 files changed, 43 insertions(+), 116 deletions(-)

diff --git a/ftpfs/dir.c b/ftpfs/dir.c
index be20b3d..bd3b30c 100644
--- a/ftpfs/dir.c
+++ b/ftpfs/dir.c
@@ -30,72 +30,33 @@
 void
 free_entry (struct ftpfs_dir_entry *e)
 {
-  assert (! e->self_p);/* We should only free deleted nodes.  
*/
   free (e->name);
   if (e->symlink_target)
 free (e->symlink_target);
   free (e);
 }
-
-/* Put the directory entry E into the hash table HTABLE, of length HTABLE_LEN. 
 */
-static void
-insert (struct ftpfs_dir_entry *e,
-   struct ftpfs_dir_entry **htable, size_t htable_len)
-{
-  struct ftpfs_dir_entry **t = [e->hv % htable_len];
-  if (*t)
-(*t)->self_p = >next;
-  e->next = *t;
-  e->self_p = t;
-  *t = e;
-}
-
-/* Replace DIR's hashtable with a new one of length NEW_LEN, retaining all
-   existing entries.  */
-static error_t
-rehash (struct ftpfs_dir *dir, size_t new_len)
-{
-  int i;
-  size_t old_len = dir->htable_len;
-  struct ftpfs_dir_entry **old_htable = dir->htable;
-  struct ftpfs_dir_entry **new_htable =
-malloc (new_len * sizeof (struct ftpfs_dir_entry *));
-
-  if (! new_htable)
-return ENOMEM;
 
-  memset (new_htable, 0, new_len * sizeof(struct ftpfs_dir_entry *));
-
-  for (i = 0; i < old_len; i++)
-while (old_htable[i])
-  {
-   struct ftpfs_dir_entry *e = old_htable[i];
-
-   /* Remove E from the old table (don't bother to fixup
-  e->next->self_p).  */
-   old_htable[i] = e->next;
-
-   insert (e, new_htable, new_len);
-  }
-
-  free (old_htable);
-
-  dir->htable = new_htable;
-  dir->htable_len = new_len;
-
-  return 0;
-}
-
-/* Calculate NAME's hash value.  */
-static size_t
-hash (const char *name)
+/* Calculate NAME_PTR's hash value.  */
+static hurd_ihash_key_t
+ihash_hash (const void *name_ptr)
 {
+  const char *name = (const char *) name_ptr;
   size_t hv = 0;
   while (*name)
 hv = ((hv << 5) + *name++) & 0xFF;
   return hv;
 }
 
+/* Compare two names which are used as keys.  */
+static int
+ihash_compare (const void *key1, const void *key2)
+{
+  const char *name1 = (const char *) key1;
+  const char *name2 = (const char *) key2;
+
+  return strcmp (name1, name2) == 0;
+}
+
 /* Lookup NAME in DIR and return its entry.  If there is no such entry, and
ADD is true, then a new entry is allocated and returned, otherwise 0 is
returned (if ADD is true then 0 can be returned if a memory allocation
@@ -103,23 +64,14 @@ hash (const char *name)
 struct ftpfs_dir_entry *
 lookup (struct ftpfs_dir *dir, const char *name, int add)
 {
-  size_t hv = hash (name);
-  struct ftpfs_dir_entry *h = dir->htable[hv % dir->htable_len], *e = h;
-
-  while (e && strcmp (name, e->name) != 0)
-e = e->next;
+  struct ftpfs_dir_entry *e =
+hurd_ihash_find (>htable, (hurd_ihash_key_t) name);
 
   if (!e && add)
 {
-  if (dir->num_entries > dir->htable_len)
-   /* Grow the hash table.  */
-   if (rehash (dir, (dir->htable_len + 1) * 2 - 1) != 0)
- return 0;
-
   e = malloc (sizeof *e);
   if (e)
{
- e->hv = hv;
  e->name = strdup (name);
  e->node = 0;
  e->dir = dir;
@@ -128,13 +80,11 @@ lookup (struct ftpfs_dir *dir, const char *name, int add)
  e->symlink_target = 0;
  e->noent = 0;
  e->valid = 0;
+  e->deleted = 0;
  e->name_timestamp = e->stat_timestamp = 0;
  e->ordered_next = 0;
  e->ordered_self_p = 0;
- e->next = 0;
- e->self_p = 0;
- insert (e, dir->htable, dir->htable_len);
- dir->num_entries++;
+  hurd_ihash_add (>htable, (hurd_ihash_key_t) e->name, e);
}
 }
 
@@ -148,28 +98,21 @@ ordered_unlink (struct ftpfs_dir_entry *e)
   if (e->ordered_self_p)
 *e->ordered_self_p = e->ordered_next;
   if (e->ordered_next)
-e->ordered_next->self_p = e->ordered_self_p;
+e->ordered_next->ordered_self_p = e->ordered_self_p;
 }
 
 /* Delete E from its directory, freeing any resources it holds.  */
 static void
 delete (struct ftpfs_dir_entry *e, struct ftpfs_dir *dir)
 {
-  dir->num_entries--;
-
-  /* Take out of the hash chain.  */
-  if (e->self_p)
-*e->self_p = e->next;
-  if (e->next)
-e->next->self_p = e->self_p;
-
   /* This indicates a deleted entry.  */
-  e->self_p = 0;
-  e->next = 0;
+  e->deleted = 1;
 
   /* Take out of the directory ordered list.  */
   ordered_unlink (e);
 
+  hurd_ihash_locp_remove (>htable, e->dir_locp);
+
   /* If there's a node attached, we'll delete the entry whenever it goes
  away, otherwise, just delete it now.  */
   if (! e->node)
@@ -180,25 +123,23 @@ delete (struct 

Re: [PATCH] Check for a return value in netfs_make_peropen before using it in netfs_make_protid.

[Flavio Cruz]

* libnetfs/trans-callback.c: Add check for netfs_make_peropen.
---
 libnetfs/trans-callback.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/libnetfs/trans-callback.c b/libnetfs/trans-callback.c
index f4f0c62..9ebe11c 100644
--- a/libnetfs/trans-callback.c
+++ b/libnetfs/trans-callback.c
@@ -58,14 +58,18 @@ _netfs_translator_callback2_fn (void *cookie1, void 
*cookie2, int flags,
   struct protid *cred;
   struct node *node = cookie1;
   struct iouser *user;
+  struct peropen *po;
 
   err = iohelp_create_simple_iouser (, node->nn_stat.st_uid,
   node->nn_stat.st_gid);
   if (err)
 return err;
 
-  cred = netfs_make_protid (netfs_make_peropen (node, flags, cookie2),
-   user);
+  po = netfs_make_peropen (node, flags, cookie2);
+  if (! po)
+return errno;
+
+  cred = netfs_make_protid (po, user);
   if (cred)
 {
   *underlying = ports_get_right (cred);

Re: [PATCH] Check for a return value in netfs_make_peropen before using it in netfs_make_protid.

[Samuel Thibault]

Flavio Cruz, on Sun 07 Feb 2016 09:19:38 -0500, wrote:
> +  po = netfs_make_peropen (node, flags, cookie2);
> +  if (! po)
> +return ENOMEM;

I'd say rather return errno.

Samuel

[PATCH] Check for a return value in netfs_make_peropen before using it in netfs_make_protid.

[Flavio Cruz]

* libnetfs/trans-callback.c: Add check.
---
 libnetfs/trans-callback.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/libnetfs/trans-callback.c b/libnetfs/trans-callback.c
index f4f0c62..9ebe11c 100644
--- a/libnetfs/trans-callback.c
+++ b/libnetfs/trans-callback.c
@@ -58,14 +58,18 @@ _netfs_translator_callback2_fn (void *cookie1, void 
*cookie2, int flags,
   struct protid *cred;
   struct node *node = cookie1;
   struct iouser *user;
+  struct peropen *po;
 
   err = iohelp_create_simple_iouser (, node->nn_stat.st_uid,
   node->nn_stat.st_gid);
   if (err)
 return err;
 
-  cred = netfs_make_protid (netfs_make_peropen (node, flags, cookie2),
-   user);
+  po = netfs_make_peropen (node, flags, cookie2);
+  if (! po)
+return ENOMEM;
+
+  cred = netfs_make_protid (po, user);
   if (cred)
 {
   *underlying = ports_get_right (cred);
-- 
2.6.4

Re: [PATCH] Check for a return value in netfs_make_peropen before using it in netfs_make_protid.

[Samuel Thibault]

Flavio Cruz, on Sun 07 Feb 2016 09:51:24 -0500, wrote:
> * libnetfs/trans-callback.c: Add check for netfs_make_peropen.

pushed, thanks!

Re: [PATCH] Check for a return value in netfs_make_peropen before using it in netfs_make_protid.

[James Clarke]

On 7 Feb 2016, at 14:51, Flavio Cruz  wrote:
> -  cred = netfs_make_protid (netfs_make_peropen (node, flags, cookie2),
> - user);
> +  po = netfs_make_peropen (node, flags, cookie2);
> +  if (! po)
> +return errno;

You need to free user.

James



signature.asc
Description: Message signed with OpenPGP using GPGMail

[PATCH] Fixed leaks in _netfs_translator_callback2_fn

[James Clarke]

* libnetfs/trans-callback.c (_netfs_translator_callback2_fn): Fixed leaking
iouser and peropen structs on error.
---
 libnetfs/trans-callback.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/libnetfs/trans-callback.c b/libnetfs/trans-callback.c
index 3f1aee6..7816bd1 100644
--- a/libnetfs/trans-callback.c
+++ b/libnetfs/trans-callback.c
@@ -67,7 +67,10 @@ _netfs_translator_callback2_fn (void *cookie1, void 
*cookie2, int flags,
 
   po = netfs_make_peropen (node, flags, cookie2);
   if (! po)
-return errno;
+{
+  iohelp_free_iouser (user);
+  return errno;
+}
 
   cred = netfs_make_protid (po, user);
   if (cred)
@@ -79,6 +82,7 @@ _netfs_translator_callback2_fn (void *cookie1, void *cookie2, 
int flags,
 }
   else
 {
+  netfs_release_peropen (po);
   iohelp_free_iouser (user);
   return errno;
 }
-- 
2.7.1

Re: [PATCH] Remove libfshelp/trans.h and libfshelp/locks.h.

[Justus Winter]

Quoting Flavio Cruz (2016-02-07 08:04:34)
> * libfshelp/locks.h: Remove.
> * libfshelp/trans.h: Remove. struct transboot is not used anywhere.
> * libfshelp/fetch-root.c: Adjust includes.
> * libfshelp/lock-acquire.c: Likewise.
> * libfshelp/lock-init.c: Likewise.

Merged, thanks!

Justus

Re: [PATCH] Use refcount_t for peropen reference counting in libnetfs.

[Justus Winter]

Quoting Flavio Cruz (2016-02-07 07:33:00)
> * libnetfs/netfs.h: Use refcount_t.
> * libnetfs/make-peropen.c: Initialize to 1 with refcount_init just like in 
> libdiskfs.
> * libnetfs/make-protid.c: Don't increment the count here. Do it like 
> libdiskfs.
> * libnetfs/io-duplicate.c: Add refcount_ref since netfs_make_protid no longer 
> increments the refcount.
> * libnetfs/io-reauthenticate.c: Likewise.
> * libnetfs/io-restrict-auth.c: Likewise.
> * libnetfs/release-peropen.c: Dereference without locking.

Merged, thanks!

Justus

Re: [PATCH] Fixed leaks in _netfs_translator_callback2_fn

[James Clarke]

> On 7 Feb 2016, at 23:10, Samuel Thibault  wrote:
> Flávio Cruz, on Sun 07 Feb 2016 23:57:25 +0100, wrote:
>> Maybe here we should do it as follows:
>> 
>> err = errno;
>> netfs_release_peropen (po);
>> iohelp_free_iouser (user);
>> return err;
> 
> Yes, you never know what they could be doing to errno.
> 
> Samuel

Does that include changing

>   po = netfs_make_peropen (node, flags, cookie2);
>   if (! po)
> -return errno;
> +{
> +  iohelp_free_iouser (user);
> +  return errno;
> +}

to

>   po = netfs_make_peropen (node, flags, cookie2);
>   if (! po)
> -return errno;
> +{
> +  err = errno;
> +  iohelp_free_iouser (user);
> +  return err;
> +}

or can we assume the free functions don’t set errno?

James


signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: [PATCH] Fixed leaks in _netfs_translator_callback2_fn

[Samuel Thibault]

James Clarke, on Sun 07 Feb 2016 23:13:06 +, wrote:
> > On 7 Feb 2016, at 23:10, Samuel Thibault  wrote:
> > Flávio Cruz, on Sun 07 Feb 2016 23:57:25 +0100, wrote:
> >> Maybe here we should do it as follows:
> >> 
> >> err = errno;
> >> netfs_release_peropen (po);
> >> iohelp_free_iouser (user);
> >> return err;
> > 
> > Yes, you never know what they could be doing to errno.
> > 
> > Samuel
> 
> Does that include changing
> 
> >   po = netfs_make_peropen (node, flags, cookie2);
> >   if (! po)
> > -return errno;
> > +{
> > +  iohelp_free_iouser (user);
> > +  return errno;
> > +}
> 
> to
> 
> >   po = netfs_make_peropen (node, flags, cookie2);
> >   if (! po)
> > -return errno;
> > +{
> > +  err = errno;
> > +  iohelp_free_iouser (user);
> > +  return err;
> > +}

I'd say so too, yes.

Samuel

Re: [PATCH] Fixed leaks in _netfs_translator_callback2_fn

[Samuel Thibault]

Flávio Cruz, on Sun 07 Feb 2016 23:57:25 +0100, wrote:
> Maybe here we should do it as follows:
> 
> err = errno;
> netfs_release_peropen (po);
> iohelp_free_iouser (user);
> return err;

Yes, you never know what they could be doing to errno.

Samuel

[PATCH v2] Fixed leaks in _netfs_translator_callback2_fn

[James Clarke]

* libnetfs/trans-callback.c (_netfs_translator_callback2_fn): Fixed leaking
iouser and peropen structs on error.
---
 libnetfs/trans-callback.c | 10 --
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/libnetfs/trans-callback.c b/libnetfs/trans-callback.c
index 3f1aee6..ed21aa2 100644
--- a/libnetfs/trans-callback.c
+++ b/libnetfs/trans-callback.c
@@ -67,7 +67,11 @@ _netfs_translator_callback2_fn (void *cookie1, void 
*cookie2, int flags,
 
   po = netfs_make_peropen (node, flags, cookie2);
   if (! po)
-return errno;
+{
+  err = errno;
+  iohelp_free_iouser (user);
+  return err;
+}
 
   cred = netfs_make_protid (po, user);
   if (cred)
@@ -79,8 +83,10 @@ _netfs_translator_callback2_fn (void *cookie1, void 
*cookie2, int flags,
 }
   else
 {
+  err = errno;
+  netfs_release_peropen (po);
   iohelp_free_iouser (user);
-  return errno;
+  return err;
 }
 }
 
-- 
2.7.1

Re: [PATCH] Fixed leaks in _netfs_translator_callback2_fn

[Flávio Cruz]

Thanks for noticing!


> @@ -79,6 +82,7 @@ _netfs_translator_callback2_fn (void *cookie1, void
> *cookie2, int flags,
>  }
>else
>  {
> +  netfs_release_peropen (po);
>iohelp_free_iouser (user);
>return errno;
>  }


Maybe here we should do it as follows:

err = errno;
netfs_release_peropen (po);
iohelp_free_iouser (user);
return err;

I've seen this pattern a few times in other files since the release calls
may reassign errno.


-- 
Flávio Cruz / flavioc...@gmail.com