pa_modargs interface question

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sun, 2016-07-03 at 21:43 +0200, Ulrich Eckhardt wrote:
> Hi!
> 
> Looking at the various functions in modargs.h and their concrete
> behaviour. What I find odd there is that the functions return an
> errorcode (an int, wouldn't it be worthwhile to use a typedef for that?)

A typedef might be good, if the error code actually contains some
useful information, but if it's just -1 for failure and 0 for success,
I don't think a typedef would be beneficial. Once you know that
failures are always reported as negative integers and successes are
always reported as non-negative integers, I don't think the readability
of the code suffers from using plain ints.

We do have pa_error_code_t that perhaps could be used as the return
type for functions that return meaningful error codes. pa_error_code_t
values are positive, though, so maybe that's not so good after all...

(I would personally prefer something like what GLib does for reporting
errors, because it allows better error messages than just a simple
error code, but maybe it's too late to change the convention at this
point. And this is only relevant when reporting errors to clients,
because at server side we can always print an informative message to
the log.)

> but that they don't always return an error when extracting an argument
> fails. In particular when the argument is not present, they return
> success nonetheless.
> 
> At first, I thought that was a bug. However, that peculiar behaviour is
> present in every pa_modargs_get_value_* function, and I rather think
> that it's intention. The intention is that the supplied pointer to the
> value is supposed to give both the default value and the output
> value, right?

Right. Considering a missing modarg an error would only cause trouble,
because almost every modarg is optional.

-- 
Tanu


[Index of Archives]     [Linux Audio Users]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux