Code review comment for lp:~xnox/upstart/fix-qemu-test

Revision history for this message
James Hunt (jamesodhunt) wrote :

Hi Dmitrijs,

I think this is still subtly racy since it is possible that even in a failure scenario a partial log could be written.

The fix I'd suggest is:

(1) Put a check immediately after the 'nih_free (class)' in the TEST_ALLOC_SAFE block that performs the following checks:

- if test_alloc_failed is true, and the log file exists, just unlink it (there is no point in checking it as it might be only a partial log - we cannot know).

- if test_alloc_failed is not true, ensure the log file exists, check its contents and unlink it (essentially the block you have added).

(2) Re-instate the original 'assert0 (rmdir (dirname))' at the end of the test.

This way, there is no possibility of:

- the tests failing to detect that a full log was somehow written in a failure scenario.
- a failure resulting from a partial log being written in one of the failure iterations, followed by a successful write in the successful iteration (recall that the logs are appended to).

review: Needs Fixing

« Back to merge proposal