[apparmor] [RFC] libapparmor: Don't expose a value-result param in aa_getpeercon_raw()

[John Johansen]

On 06/25/2013 02:58 PM, Seth Arnold wrote:
> On Tue, Jun 25, 2013 at 01:19:17PM -0700, Tyler Hicks wrote:
>> Unify aa_getprocattr_raw() and aa_getpeercon_raw() function prototypes
>> by hiding the len value-result parameter.
>>
>> Signed-off-by: Tyler Hicks <tyhicks at canonical.com>
>> Cc: Seth Arnold <seth.arnold at canonical.com>
>> Cc: John Johansen <john.johansen at canonical.com>
>> ---
>>
>> Code speaks louder and more clearly than my poor explanation, so here's an RFC
>> patch on the idea.
>>
>> I think that this addresses Seth's distaste of the len parameter. It makes the
>> libapparmor API a little more predictable, but hides a useful feature of the
>> getsockopt() syscall which will make external callers guess at buffer
>> reallocation sizes (but they already do that with aa_getprocattr_raw()).
>>
>> I'm fine with or without this patch, as I don't expect (m)any applications to
>> use aa_getpeercon_raw().
> 
> Hrm, I thought I was going to like this more than I did. I thought
> unifying the interface to match aa_getprocattr_raw() was going to be a
> good idea, but it just doesn't feel very satisfying.
> 
> Incidentally, because I also failed to describe what I thought would
> make a good patch, here's what I thought would work:
> 
> diff -Naurp libapparmor.orig/src/kernel_interface.c libapparmor/src/kernel_interface.c
> --- libapparmor.orig/src/kernel_interface.c	2012-12-06 18:33:20.000000000 -0800
> +++ libapparmor/src/kernel_interface.c	2013-06-25 14:33:45.000000000 -0700
> @@ -554,7 +554,7 @@ int aa_getcon(char **con, char **mode)
>   * @con: pointer to buffer to store confinement string
>   * @size: initially contains size of the buffer, returns size of data read
>   *
> - * Returns: length of confinement data including null termination or -1 on error
> + * Returns: 0 on success or -1 on error
>   *          if errno == ERANGE then @size will hold the size needed
>   */
>  int aa_getpeercon_raw(int fd, char *buffer, int *size)
> 
> 
> BUT, enough other routines return the length via return that having this
> one be different would also be annoying.
> 
> I assume it's too late to add a & to the aa_getprocattr_raw() len
> parameter? :)
> 
sigh yes. Consistency would be nice, but not breaking/changing an existing shipped api
is even better.