[PATCH 2/6] nf: nfnl_*_str2copy_mode() should return int

[Thomas Haller]

On Tue, 2014-08-26 at 11:21 +0100, Thomas Graf wrote:
> On 08/26/14 at 12:03pm, Thomas Haller wrote:
> > On Tue, 2014-08-26 at 01:09 +0200, Thomas Graf wrote:
> > > ... to be able to return a negative error code for unknown modes.
> > > 
> > > Signed-off-by: Thomas Graf <tgraf at suug.ch>
> > > ---
> > >  include/netlink/netfilter/log.h   | 2 +-
> > >  include/netlink/netfilter/queue.h | 2 +-
> > >  lib/netfilter/log_obj.c           | 2 +-
> > >  lib/netfilter/queue_obj.c         | 2 +-
> > >  src/nf-log.c                      | 2 +-
> > >  src/nf-queue.c                    | 2 +-
> > >  6 files changed, 6 insertions(+), 6 deletions(-)
> > 
> > 
> > isn't this an ABI break?
> 
> Strictly speaking it is. We should be fine unless we compile
> with -fshort-enums though.
> 
> I should have mentioned this in the commit message actually:
> assuming that enum can translate to anything, I can't see how
> a caller could correctly check for the -1 error condition.
> Even if we require the caller to cast, what do we have him cast
> it to?

shouldn't the correct way be:

if (value == (enum nfnl_queue_copy_mode) -1)
   ...


> So either way, we are breaking the ABI in a minor way.
> 
> Thoughts?

given that the enum size depends on compiler/compiler options
(-fshort-enums), it seems wrong to have them as function
arguments/return values in first place.

for that reason, we should get rid of enums for
that purpose.


Thomas
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part
URL: <http://lists.infradead.org/pipermail/libnl/attachments/20140826/8fc5ac6f/attachment.sig>