Re: [PATCH 0/3] avoiding unintended consequences of git_path() usage
- From
Michael Haggerty <mhagger@alum.mit.edu>
- Date
- Nov 16, 2011, 13:44 UTC
- Message-ID
- <4EC3BE53.3020705@alum.mit.edu>
- In-Reply-To
- <CACsJy8A2=qBiyY3SD-PZo+E=U+Dfjm1UQidgq6khQARZ3d41WQ@mail.gmail.com>
On 11/16/2011 02:33 PM, Nguyen Thai Ngoc Duy wrote:
Show 11 quoted lines
> 2011/11/16 Jonathan Nieder <jrnieder@gmail.com>: >> Jokes aside, here's a rough series to do the git_path -> >> git_path_unsafe renaming. While writing it, I noticed a couple of >> bugs, hence the two patches before the last one. Patch 2 is the more >> interesting one. > > Another approach is do nothing and leave it for a static analysis tool > to detect potential problems. I'm looking at sparse at the moment, > although I know nothing about it to say if it can or cannot detect > such problems. We can at least make sparse detect return value from > git_path() being passed to an unsafe function, I think.
For the cases when static analysis doesn't suffice, recently I posted some patches that make it possible for debug a problem that results from the use of a "stale" buffer [1]. But having myself also been bitten by this problem, I'd also be in favor of a more systematic solution, even if it has a small runtime cost. After all, most of the time the filename created by git_path() is going to be passed to the kernel a moment later, which will usually be vastly slower than an extra malloc/free.
Michael
[1] http://comments.gmane.org/gmane.comp.version-control.git/182209
-- Michael Haggerty mhagger@alum.mit.edu http://softwareswirl.blogspot.com/