[cairo] New API proposal: cairo_restore_status()

[Andrea Canciani]

A discussion on IRC pointed out the usefulness of some way to reset
the error status of a cairo_t.

Allowing to reset it directly is not likely to be a good idea, because its
state might be invalid or inconsistent. To avoid this issue, it is possible
to only allow resetting the status along with a valid state.

The attached patches provide an implementation of cairo_restore_status():

+ * cairo_restore_status:
+ * @cr: a #cairo_t
+ *
+ * Restores @cr to the state saved by a preceding call to cairo_save()
+ * and removes that state from the stack of saved states. The status
+ * of @cr will be reset to the same value it was in before calling
+ * cairo_save().
+ **/
+void
+cairo_restore_status (cairo_t *cr)

The implementation idea and an use case come from Benjamin Otte, but
it has already been suggested in the ML multiple times.

The rationale for not providing a way to reset the error status
"[...] is that we don't have any
guarantee that we can restore the context to any particular state
after an error occurs. Without a huge audit and a lot of pain to make
every cairo operation atomic, the only conceivable state to restore to
would be the default state. And if we had an operation that did
recover-error-and-return-context-to-default-state then I would argue
that that wouldn't achieve anything that cairo_destroy;cairo_create
does not also achieve, but that it could lead to confusion as people
might not expect all of the state to be reset when recovering from
errors."
(from http://lists.freedesktop.org/archives/cairo/2006-September/007892.html )

It appears that implementing such a feature is not as hard and
painful as it was foreseen.

The main idea is that *conceptually* the status is moved from the cairo_t to
the state and cairo_state() always pushes a state, even if it cannot allocate
memory. In this model, cairo_restore_status() just removes the top state
(thus it restores the status of the previous state), while cairo_restore()
removes the top state, but also copies the status from it to the previous
state.

This cannot cause any observable difference in existing applications,
because cairo_save() and cairo_restore() keep exactly their existing
semantic. It should not even cause bad interactions with libraries
which decide to use cairo_restore_state(), as long as the bracketing
save/restore is preserved correctly.
It might cause some changes in the behavior of applications which
call cairo_save() and then use a library function which was at first
supposed to call cairo_restore() and then was changed to use
cairo_restore_status().

Examples:
OK:
main()
{
cairo_save();
foo();
cairo_restore(); /* this can be changed to cairo_restore_status() safely */
}

BROKEN:
lib_func_which_calls_cairo_restore()
{
change_the_current_state();
cairo_restore(); /* this CANNOT be changed to cairo_restore_status() safely */
}

main()
{
cairo_save();
lib_func_which_calls_cairo_restore();
}

WORKAROUND:
lib_func_which_calls_cairo_restore()
{
/* Adding a pair of internal brackets, solves the observability problem */
cairo_save();
change_the_current_state();
cairo_restore_status();
cairo_restore(); /* this CANNOT be changed to cairo_restore_status() safely */
}

main() {
cairo_save();
lib_func_which_calls_cairo_restore();
}


Details about why the implementation is correct (without the huge audit)
follow:

 - a count of virtual failure states is kept in the cairo_t. cairo_save() on an
   cr with an invalid status just increments the number of virtual
stack elements.
   cairo_restore() on a cr with virtual stack elements just decrements this
   number. This allows counting the stack elements without performing
   allocation, so it is always successful.

 - cairo_save/restore*() never modify a cr with an "excessive" number of
   states in the stack. This allows a very clean workaround to operations
   on nil cairo_t's and solves the problems that might arise if the counter
   for virtual states wrapped around.

 - real operations (not just counter incs/decs) are only performed on cr's
   whose failure states count is 0

ASSUMPTION: _cairo_gstate_save() can only fail because it cannot
   allocate memory for the new state and in this case it will not modify
   the current stack in any way.

 - cairo_save() will either manage to create a new state and push it on
   the top of the stack (resulting in a stack of valid states) or it will not
   have enough memory to perform the allocation. In the latter case it
   will push a virtual invalid state (i.e. set the failure count to 1 and the
   status to an error state)

ASSUMPTION: _cairo_gstate_restore() can only fail if the stack is just
   the current state.

 - cairo_restore() will simply restore the previous state, polluting it with
   the current status (if it is an error)
NB: the new implementation reflects this: cairo_restore() now removes
   the top state even if the cr is in an error status

 - cairo_restore_status() will restore the previous state, including the
   status. If the failure states counter is 0, the status of the previous
   state was obviously CAIRO_STATUS_SUCCESS.

 - cairo_restore*() always set the cr to an invalid status if the pop
   operation on the stack is not legitimate

Would this be an acceptable addition for 1.12?
Should (some of) this documentation be added as comments to the
actual code? Would adding the assumptions (and/or asserting them)
be sufficient?

Andrea