[pulseaudio-discuss] [PATCH] add section about error handling to doxygen documentation

[Peter Meerwald]

Hello Tanu,

> > + * The PulseAudio API indicates error conditions by returning an integer
> > + * value -1 or a NULL pointer. On success, an integer value 0 or a valid
> > + * pointer is returned. Certain function provide an optional error code
> > + * (see #pa_error_code) which can be turned into a human readable message
> > + * using pa_strerror().

> This text promises too much about the return codes. Yes, -1 and 0 are
> the most common values, but the numbers shouldn't be hardcoded in the
> user code ("user" as in API user). A negative return code means failure
> and non-negative means success, thus the user should always use "< 0" or
> ">= 0" to check for failure/success. If there is a special meaning
> attached to some individual numbers, that will be documented separately.

currently there is no documentation that I am aware of on what a 
return code could mean 

the text can be amended to talk about negative and non-negative return 
codes instead of -1 and 0 -- however, -1 is at the moment mentioned 
specifically for several API calls

I agree that special cases should be documented separately as needed;
however, this is missing in certain places: e.g. pa_proplist_unset() and 
pa_proplist_contains() -- I suggest patches for those two

> Another thing is the question of when pa_strerror() can be used. That
> should be documented unambiguously. I don't know what the current
> situation is - my guess would be that the documentation is lacking in
> this regard.

I was not aware of this problem and simply assumed that an error code can 
be passed to pa_strerror()

> I think this text should either say what it says now, or that
> pa_strerror() can be used always unless otherwise stated. Which one is
> the right choice depends on which is more common, those error codes that
> can be passed to pa_strerror() or those that can't (I haven't checked
> what is the situation).

/** Return a human readable error message for the specified numeric error code */
const char* pa_strerror(int error);

here no indication what a numeric error code is supposed to be; the API 
uses 'int' and not a 'pa_error_code_t' (which I suggest to introduce for a 
different reason)

in def.h you find:
/** Error values as used by pa_context_errno(). Use pa_strerror() to 
convert these values to human readable strings */

strictly speaking, this would restrict pa_strerror() to values returned by 
pa_context_errno()

however, the example pacat-simple.c does the following:
 if (!(s = pa_simple_new(NULL, argv[0], PA_STREAM_PLAYBACK, NULL, "playback", &ss, NULL, NULL, &error))) {
        fprintf(stderr, __FILE__": pa_simple_new() failed: %s\n", pa_strerror(error));

I have not spotted any error codes not defined in defs.h in a brief review

> Regardless of what is the current situation, I think it would make sense
> to guarantee every error code in the public API to be suitable to be
> passed to pa_strerror().

I fully agree, this sounds sensible
I don't see a reason to have error codes as int and not enums -- but I 
guess that would be a too bit change at the moment

regards, p.

-- 

Peter Meerwald
+43-664-2444418 (mobile)