On 10/24/2011 10:49 AM, Jeremy Huddleston wrote:
The module which opens the fd should be the same module that closes it.  
Letting that cross between the common/specific boundary seems problematic.  I'd 
prefer to see a new hook added for implementation-specific cleanup, and the 
close() live in linux_sysfs.c itself.  That will allow for better abstraction 
down the road as other implementations might want to do something similar.

Sounds good. I'll send another patch shortly.

Thanks,
Nithin.



On Oct 24, 2011, at 10:18, Nithin Sujir wrote:


On 10/24/2011 09:51 AM, Jeremy Huddleston wrote:
While possibly safe in practice, this doesn't look like the correct fix.  It is 
possible that this will result in a close(0) if HAVE_MTRR is defined and 
mtrr_fd was just never set.

HAVE_MTRR is defined if<asm/mtrr.h>   exists.
mtrr_fd is set if HAVE_MTRR is defined and linux_sysfs is used.

Probably a trivial example of this would be to "sudo touch 
/usr/include/asm/mtrr.h" on FreeBSD.

That is a valid point. I don't have a freebsd system to test it but based on 
code review I agree that what you say will happen.

Would you suggest adding an #ifdef linux around the close or since the pci_sys 
structure is allocated with a calloc either directly or indirectly, I can change 
the condition to check for>  0.

Thanks,
Nithin.



On Oct 21, 2011, at 11:49, Nithin Nayak Sujir wrote:

Since the fd is not closed, calling pci_system_init and
pci_system_cleanup more than 1024 times results in "too many files open"
error.

Signed-off-by: Nithin Nayak Sujir<nsu...@broadcom.com>
---
src/common_init.c |    6 ++++++
1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/src/common_init.c b/src/common_init.c
index 5e91c27..d7ade3f 100644
--- a/src/common_init.c
+++ b/src/common_init.c
@@ -31,7 +31,9 @@

#include<stdlib.h>
#include<errno.h>
+#include<unistd.h>

+#include "config.h"
#include "pciaccess.h"
#include "pciaccess_private.h"

@@ -122,6 +124,10 @@ pci_system_cleanup( void )
        (*pci_sys->methods->destroy)();
     }

+#ifdef HAVE_MTRR
+    if (pci_sys->mtrr_fd != -1)
+           close(pci_sys->mtrr_fd);
+#endif
     free( pci_sys );
     pci_sys = NULL;
}
--
1.7.1


_______________________________________________
xorg@lists.freedesktop.org: X.Org support
Archives: http://lists.freedesktop.org/archives/xorg
Info: http://lists.freedesktop.org/mailman/listinfo/xorg
Your subscription address: jerem...@freedesktop.org







_______________________________________________
xorg@lists.freedesktop.org: X.Org support
Archives: http://lists.freedesktop.org/archives/xorg
Info: http://lists.freedesktop.org/mailman/listinfo/xorg
Your subscription address: arch...@mail-archive.com

Reply via email to