On Wed, Feb 26, 2020 at 04:20:50PM +0100, Tim Duesterhus wrote:
> Currently unique IDs for a stream are generated using repetitive code in
> multiple locations, possibly allowing for inconsistent behavior.
Just a few minor coding style comments :
> +int stream_generate_unique_id(struct stream *strm, struct list *format) {
Please put the function's opening brace on its own line. The reason for
this is when you have many arguments and variables, it easily becomes a
mess where you cannot always visually tell which ones belong to what.
> + if (strm->unique_id != NULL) {
> + return strlen(strm->unique_id);
> + }
> +
> + if ((strm->unique_id = pool_alloc(pool_head_uniqueid)) == NULL)
> + return -1;
Please avoid assignments in "if" conditions. There are two reasons for
this, both related to debugging:
- if you want to quickly disable the error check or put a lock
around or whatever, you cannot without splitting the line in
two ;
- you often cannot single-step through it in a debugger or put a
breakpoint after the pool_alloc() call.
Thanks,
Willy
