Use RING_FOREACH_SAFE in red_channel.c functions which are missing it

Currently, both red_channel_pipes_add_type() and
red_channel_pipes_add_empty_msg() use plaing RING_FOREACH() which is not
safe versus removals from the ring within the loop body.

Although it's rare, such a removal can occur in both cases.  In the case
of red_channel_pipes_add_type() we have:
    red_channel_pipes_add_type()
    -> red_channel_client_pipe_add_type()
        -> red_channel_client_push()

And in the case of red_channel_client_pipes_add_empty_msg() we have:
    red_channel_client_pipes_add_empty_msg()
    -> red_channel_client_pipe_add_empty_msg()
        -> red_channel_client_push()

But red_channel_client_push() can cause a removal from the clients ring if
a network error occurs:
    red_channel_client_push()
    -> red_channel_client_send()
        -> red_peer_handle_outgoing()
            -> handler->cb->on_error callback
            =  red_channel_client_default_peer_on_error()
                -> red_channel_client_disconnect()
                    -> red_channel_remove_client()
                        -> ring_remove()

When this error path does occur, the assertion in RING_FOREACH()'s
ring_next() trips, and the process containing the spice server is aborted.
i.e. your whole VM dies, as a result of an unfortunately timed network
error on the spice channel.

Please apply.

Signed-off-by: David Gibson <dgibson@redhat.com>
red_worker: use only RCC_FOREACH_SAFE

RCC_FOREACH may be dangerous

The following patches replace FOREACH loops with a SAFE version.
Using unsafe loops may cause spice-server to abort (assert fails).
Specifically a read/write fail in those loops, may cause the client
to disconnect, removing the node currently iterated, which cause spice
to abort in ring_next():
 -- assertion `pos->next != NULL && pos->prev != NULL' failed
red_worker: reuse DCC_FOREACH in WORKER_DCC_FOREACH

The only thing that is needed is to get the channel out of the worker.
red_worker: make WORKER_FOREACH_DCC safe

Specifically, the loop in red_pipes_add_draw can cause spice to abort.

In red_worker.c (WORKER_FOREACH_DCC):
  red_pipes_add_drawable
    red_pipe_add_drawable
      red_handle_drawable_surfaces_client_synced
        red_push_surface_image
          red_channel_client_push
            red_channel_client_send
              red_peer_handle_outgoing
                reds_stream_writev (if fails -- EPIPE)
                handler->cb->on_error = red_channel_client_disconnect()
                  red_channel_remove_client()
                  ring_remove() -- of rcc from channel.clients ring.
red_worker: use only DRAWABLE_FOREACH_GLZ_SAFE
red_worker: make DRAWABLE_FOREACH_DPI safe
red_worker: delete unused CCC_FOREACH
red_worker: use a generic SAFE_FOREACH macro

Introduce SAFE_FOREACH macro

Make other safe iterators use SAFE_FOREACH
red_channel: replace RING_FOREACH with RING_FOREACH_SAFE in some places

This was originally intended to fix the problem fixed by
commit 53488f0275d6c8a121af49f7ac817d09ce68090d.

What is left are FOREACH loops that are at less risk and maybe safe (no
read/write or disconnect/destroy are called from within them).
server/tests: test_display_base: set rect according to appropriate surface

When surface_id == 0, primary is used.
Otherwise (currently 1), secondary is used.

Also, remove unused test_width and test_height.
Since commit caea7699434c20dceef8fc79d21b8eeb663fbf53,
test->width and test->height are used.
server/tests: fix produce_command for create surface

Earlier in this function, test->target_surface is set to 1, which
is the only allowed non-primary surface currently.

If surface parameters are given (and specifically data is checked)
they are being used, otherwise a default surface is used.

Earlier in this function, "command" is set to a non-NULL value.
Thus, the else part was unreachable code, which is fixed now.
server/tests: invalid-option: print the bad argument

optind points to the next argument to parse.
server/tests: remove option from usage if AUTOMATED_TESTS is not configured
server/tests: test_display_width_stride: add destroy command

Otherwise, the test exits after the first iteration over all tests,
on the second attempt to create an already created surface.
server/tests: fix timer for test_empty_success
syntax-check: fix avoid_if_before_free
syntax-check: fix cast_of_argument_to_free

In this case, make syntax-check is wrong, and we actually do
need the cast.
A cast is needed when types are   uint64_t <--> pointer

Using a local "ptr" variable makes both gcc and syntax-check happy.
syntax-check: update AUTHORS
syntax-check: s/the the/the/ in a comment
syntax-check: fix no-newline or empty line at EOF