On 27 August 2017 at 12h56, Tanu Kaskinen wrote: Hi Tanu,
Thank you for your review :) > We don't have a whole lot of functions that need to both allocate a > new object and report the reason on failure, but there is some > precedent. At least pa_sink_input_new() and pa_source_output_new() > are this kind of functions, and they return the two things > differently than pa_module_load() in your patch. I think we should be > consistent, so pa_module_load() should return the error code as the > function return value, and the first argument of pa_module_load() > should be a pointer to a pa_module pointer. I hesitated between both methods, should have chosen the other one :) > The error codes should be from the pa_error_code_t enumeration. OK, great ! > pa_module_load() starts with this: > > errcode = -EIO; /* default error */ > > However, you always set errcode explicitly in every error case (which > is good), so this line is not needed, and it will also prevent the > compiler from issuing a warning if setting errcode is forgotten when > adding a new error case. Indeed. I'll cook up a new patch and send it back quite soon! -- Colin
pgpJszKv03ta4.pgp
Description: OpenPGP digital signature
_______________________________________________ pulseaudio-discuss mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
