Re: [PATCH] statd: not unlinking host files

[Steve Dickson <SteveD@redhat.com>]

Chuck Lever wrote:
> On Dec 8, 2008, at Dec 8, 2008, 6:33 PM, J. Bruce Fields wrote:
>> On Sat, Dec 06, 2008 at 09:34:25AM -0500, Steve Dickson wrote:
>>> Again working with the state file code, I notice that statd was
>>> not unlinking hosts files when the kernel sent up the
>>> sm_unmon messages. This following patch address the reason why...
>>>
>>> Comments?
>>
>> Bizarre--thanks for catching that.
>>
>> But it looks like these are actually the only two callers to xunlink?
>> In which case, we should just ditch the "check" parameter entirely and
>> avoid some confusion....
> 
> It looks like xunlink() doesn't check error returns properly either. 
> alloca() is a convenience, but the price is a SIGSEGV if the stack frame
> can't be extended.
> 
> For a system-level daemon like rpc.statd, I would rather see a proper
> implementation of this using malloc(3) or automatic storage, and
> ensuring that sprintf doesn't overrun its buffer.  This also makes it
> possible to track the buffer allocation here using valgrind.  alloca()
> is a completely inline implementation, according to its man page.
> 
> I'm not sure why statd has it's own implementation of xstrdup and
> xmalloc here as well; support/nfs/* already has both of these.  It would
> be worth getting rid of these too.
Added to the TODO list... 

steved.