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

[John Johansen]

On 06/25/2013 02:02 PM, Tyler Hicks wrote:
> On 2013-06-25 13:58:19, John Johansen wrote:
>> On 06/25/2013 01:19 PM, 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().
>>>
>> NAK
>>
>> aa_getpeercon_raw is a shipped interface, we are not changing it with out a real
>> good reason.
>>
>> Taking away a useful feature because its a little odd isn't a good enough reason.
>>
>> Nor is this patch correct as aa_getpeercon actually reallies on *len being
>> set in the error case to do its buffer dynamic buffer allocation. With this
>> patch it tries once and if it fails that is it even if it could have the
>> proper size.
> 
> This is incorrect. This patch changes aa_getpeercon() to use the
> internal getpeercon(), which does set *len upon error.
> 
ah yes, not sure how I missed that

> However, I'm not arguing for this patch. I'm just trying to get
> sarnold's ack so I can move forward with this patch set (with or without
> this RFC patch). :)
> 
well how about I give you my NAK on this one, and my ACK for the original
patch