[PATCH] libpciaccess: close mtrr fd on pci_cleanup

[Nithin Sujir]

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<nsujir at 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 at lists.freedesktop.org: X.Org support
>> Archives: http://lists.freedesktop.org/archives/xorg
>> Info: http://lists.freedesktop.org/mailman/listinfo/xorg
>> Your subscription address: jeremyhu at freedesktop.org
>>
>
>