Code review comment for lp:~jamesodhunt/upstart/bug-1199778

Revision history for this message
Dimitri John Ledkov (xnox) wrote :

On 11 July 2013 15:41, Steve Langasek <email address hidden> wrote:
> Dmitrijs, the only NIH_LIST_EMPTY() assertion that shows up as part of this patch is one that's being *removed*. Are you concerned that the other assertions are latent bugs (incorrect assertions), like this one apparently was? If not, can I suggest that review of those should be postponed to later, outside of landing this crasher fix?

Correct. Agreed same with james on #upstart.

One extra nitpick, the serialisation file needs grabbing with a
session running and session jobs/confsources and committed as a unit
test file for deserialisation. Cause, unit test that does
deserialisation of the dump should assert and fail just like the
upstart did in the bug report under previous code. This would also
allow testing whether the SRU was affected or not.

Regards,

Dmitrijs.

« Back to merge proposal