On Mon, 2017-10-30 at 17:43 +0530, Arun Raghavan wrote: > On Mon, 30 Oct 2017, at 12:45 AM, Tanu Kaskinen wrote: > > Coverity complained about data->sink being possibly NULL when it's > > dereferenced later. It was difficult for me to figure out whether that > > was a false positive or not. Hopefully the comments make it a bit > > easier to reason about the code in the future. > > > > It might be a good idea to set data->req_formats early, so that it's > > always set when setting the sink for a stream. Currently, if the > > application doesn't use the new format negotiation API, req_formats is > > set according to the sample spec at a very late stage. That means that > > sometimes data->sink gets set after data->req_formats, and sometimes > > data->req_formats gets set after data->sink, which makes it difficult to > > follow the code. > > It's hard to deterministically set it earlier than pa_sink_input_new(), > unless we make it a contract that it is set before that call for all > callers. Or did you mean we should set it early just for the > protocol-native case? I think we could have a rule saying that if pa_sink_input_new_data_set_sink() is called, that must happen after calling pa_sink_input_new_data_set_formats() or pa_sink_input_new_data_set_sample_spec() (set_sample_spec() could internally call set_formats()). It's already mandatory to set the requested formats or the sample spec before calling pa_sink_input_new(), so it can't happen that the sink input implementor would only set the sink and not the formats. The implementor just has to order the calls the right way. I applied this patch now without the second paragraph in the commit message. -- Tanu https://www.patreon.com/tanuk
