[vlc-devel] [PATCH 00/12] Make clean-up functions behave like free

[Filip Roséen]

The patches in this patch-batch allows for (the affected) clean-up functions to
safely be called with a pointer-to-NULL, effectivelly removing one-statement
if-branches to conditionally invoke such function.

Just like `free( NULL )` is safe, it would be very pleasant to see that one
could do the same for, as an example, `stream_Delete`.

Rationale
---------

These patches might be a little contriversal given the age of the codebase,
but when working across the different modules I noticed a very common pattern.

    % grep -RE _Delete -B1 --include \*.c | grep if\( -A1 -B1

    modules/stream_out/transcode/audio.c-    if( id->p_decoder->p_description)
    modules/stream_out/transcode/audio.c:        vlc_meta_Delete( id->p_decoder->p_description );
	--
	modules/access/bd/bd.c-    if( p_sys->p_m2ts )
	modules/access/bd/bd.c:        stream_Delete( p_sys->p_m2ts );
	modules/access/bd/bd.c-    if( p_sys->p_parser )
	modules/access/bd/bd.c:        stream_Delete( p_sys->p_parser );
	--
	modules/access/vdr.c-    if( p_sys->p_meta )
	modules/access/vdr.c:        vlc_meta_Delete( p_sys->p_meta );
	--
	${ several more results like the above }
	

I must also admit that I myself got bitten by the fact that many programmers
(including myself) assume clean-up functions that accepts a pointer to be NOP
if the argument passed is `NULL` (just like `free(void*)` from `<stdlib.h>`).

Legacy code
-----------

This patch will not break any previous usage of the functions in question
since it was previously undefined-behavior if the argument was NULL.

It will also not change any behavior of already written code, unless such relies
on undefined-behavior (in which case it should be burnt with fire in either
case).

Future development
------------------

One fear regarding this patch-batch, is of course that future developers sees
the documentation for the functions affected by this patch, and then assumes
that the same holds for other functions (not affected by this patch) with
similar names.

Applying doxygen comments to functions that currently lack such could be a way
of preventing the above from happening, as well as trying to propagate this
behavior (if accepted) to similar functions as soon as possible.

If accepted a I can take it upon myself to do further research, and apply
similar changes to other parts of the codebase.


Filip Roséen (12):
  vlc: vlc_access_Delete: do nothing if argument is NULL
  vlc: demux_Delete: do nothing if pointer is NULL
  vlc: filter_DeleteBlend: do nothing if argument is NULL
  vlc: stream_Delete: do nothing if pointer is NULL
  vlc: update_Delete: do nothing if argument is NULL
  vlc: vlm_Delete: do nothing if argument is NULL
  vlc: xml_Delete: do nothing if the argument is NULL
  vlc: vlc_meta_Delete: do nothing if argument is NULL
  vlc: filter_chain_Delete: do nothing if argument is NULL
  vlc: input_item_node_Delete: do nothing if argument is NULL
  vlc: xml_ReaderDelete: do nothing if argument is NULL
  vlc: subpicture_Delete: corrected broken documentation + implementation

 include/vlc_access.h     |  3 +++
 include/vlc_demux.h      |  9 ++++++++-
 include/vlc_filter.h     |  6 ++++++
 include/vlc_input_item.h |  4 ++++
 include/vlc_meta.h       |  7 +++++++
 include/vlc_stream.h     |  7 +++++++
 include/vlc_subpicture.h |  5 ++++-
 include/vlc_update.h     |  7 +++++++
 include/vlc_vlm.h        |  9 ++++++++-
 include/vlc_xml.h        | 16 +++++++++++++++-
 src/input/access.c       |  3 +++
 src/input/demux.c        |  3 +++
 src/input/item.c         |  3 +++
 src/input/meta.c         |  4 ++++
 src/input/stream.c       |  3 +++
 src/input/vlm.c          |  3 +++
 src/misc/filter.c        |  3 +++
 src/misc/filter_chain.c  |  3 +++
 src/misc/subpicture.c    |  3 +++
 src/misc/update.c        |  5 ++++-
 src/misc/xml.c           |  6 ++++++
 21 files changed, 107 insertions(+), 5 deletions(-)

-- 
2.7.3