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 -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 801 bytes Desc: OpenPGP digital signature URL: <https://lists.freedesktop.org/archives/pulseaudio-discuss/attachments/20170827/e7fbb410/attachment.sig>
