Re: [Crash-utility] Re: [RFC] Patch for fixing 'files' command: TAKE 4

On Tue, Jan 03, 2006 at 04:28:36PM -0500, Dave Anderson wrote: > Dave Anderson wrote: > > > This patch-to-your-patch seems to fix it: > > > > RCS file: /nfs/projects/cvs/crash/filesys.c,v > > retrieving revision 1.45 > > diff -r1.45 filesys.c > > 2015c2015,2016 > > < ulong files_struct_addr, fdtable_addr; > > --- > > > ulong files_struct_addr; > > > ulong fdtable_addr = 0; > > 2153c2154,2155 > > < if (!fdtable_addr || !files_struct_addr || max_fdset == 0 || max_fds == 0) { > > --- > > > if ((VALID_MEMBER(files_struct_fdt) && !fdtable_addr) || > > > !files_struct_addr || max_fdset == 0 || max_fds == 0) { > > > > With fdtable_addr uninitialized, it only gets past the "if" statement > > above on a pre-2.6.14 kernel if there were non-zero garbage left there > > on the stack. If there happened to be a zero there, the "if" path is > > taken inadvertently, and it just prints "No open files". > > > > But do you agree with my logic of the enclosed checking of the validity first > > and then the fdtable_addr? Seems right. > > > > Dave > > Hi Rachita, > > AFAICT, if we're running 2.6.14 with fdtables, it doesn't appear > that there's any possibility of fdtable_addr ever being zero? > It begins life in alloc_files() pointing to the embedded fdtable > in the files_struct, and potentially can be reassigned to a > replacement fdtable allocated from expand_fdtable(). But I don't > see anywhere that it could be cleared. > > For that matter, my original check for a NULL files_struct_addr > also looks to be impossible, even in the case of kernel threads. > Maybe back in the 2.2 timeframe it could be NULL -- I just don't > remember... > > In any case, I prefer to leave the code as it is above -- just to > be absolutely safe. > Hi Dave You are right. We need to do the validity check on files_struct_fdt before we check for fdtable_addr. I was wondering if you want me to regenerate the patch with this check..