On Friday, May 25, 2012, Peter Geoghegan wrote:

> On 25 May 2012 14:13, Magnus Hagander <mag...@hagander.net <javascript:;>>
> wrote:
> > Here's a patch that does the two easy fixes:
> > 1) writes the file to a temp file and rename()s it over the main file
> > as it writes down. This removes the (small) risk of corruption because
> > of a crash during write
> >
> > 2) unlinks the file after reading it. this makes sure it's not
> > included in online backups.
>
> Seems reasonable. It might be better to consistently concatenate the
> string literals PGSS_DUMP_FILE and ".tmp" statically. Also, I'd have
> updated the string in the errmsg callsite after the "error" tag too,
> to refer to the tmp file rather than the file proper. Forgive the
>

Agreed on the first one, and oops-forgot on the second one.


> pedantry, but I should mention that I believe that it is project
> policy to not use squiggly parenthesis following an if expression when
> that is unnecessary due to there only being a single statement.
>

Good point too - I had some other code there as well during testing, and
didn't clean it up all the way. Thanks for pointing it out!

Will apply with those fixes.




-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/

Reply via email to