Re: RFC: libkern version of inet_ntoa_r

2012-07-30 Thread Lev Serebryakov
Hello, Arnaud.
You wrote 30 июля 2012 г., 2:30:21:

  It looks very gcc-ish.
AL could you back your point with a technical argument, please ? This
AL sounds rather FUD'ish so far.
AL The ({ ... }) is nothing more than a primary-expression enclosing a
AL compound-statement;
  And how will it return value? It is completely illegal for compound
statement to return value. And to be a part of primary expression, too.

  Here is simple test:

=
1  #include stdlib.h
2  #include stdio.h
3
4  int fn(int x, int y) { return x + y; }
5
6  #define fn(x) ({ fn(x, 42); })
7
8  int main(int argc, char *argv[]) {
9  printf(%d\n, fn(10));
10 return 0;
11 }
=

  gcc compiles it and prints 52, but MSVC++ 10.0 cannot compile this
at all, and it is in its own right:

=
D:\home\lev\testcl test.c
Microsoft (R) 32-bit C/C++ Optimizing Compiler Version 16.00.40219.01 for 80x86
Copyright (C) Microsoft Corporation.  All rights reserved.

test.c
test.c(9) : error C2059: syntax error : '{'
test.c(9) : error C2059: syntax error : ')'
test.c(9) : error C2059: syntax error : ')'
test.c(10) : error C2059: syntax error : 'return'
test.c(11) : error C2059: syntax error : '}'
=

See http://gcc.gnu.org/onlinedocs/gcc/Statement-Exprs.html -- it is
clearly marked as GNU C construct, not ANSI/ISO C/C++ one.

And even more:

=
 gcc -Wall -ansi -pedantic -std=c99 test.c
test.c: In function 'main':
test.c:9: warning: ISO C forbids braced-groups within expressions

=

-- 
// Black Lion AKA Lev Serebryakov l...@freebsd.org

___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org


Re: RFC: libkern version of inet_ntoa_r

2012-07-29 Thread Luigi Rizzo
On Sat, Jul 28, 2012 at 10:14:10PM +, Bjoern A. Zeeb wrote:
 On Wed, 25 Jul 2012, Luigi Rizzo wrote:
 
 During some ipfw/dummynet cleanup i noticed that the libkern version of
 inet_ntoa_r() is missing the buffer size argument that is present in
 the libc counterpart.
 
 Any objection if i fix it ?
 
 And why exactly would you need it?  What does libc do with it?  Render
 partial IPv4 addresses?

Thanks for raising the issue because things are actually simpler
than i thought.


In general, having a function with same name and different
arguments/behaviour in the kernel and userspace is annoying and
should be avoided/fixed if possible.

We have to live with malloc/free as they are too widely used to suggest
a change, but this inet_ntoa_r() is very seldom used.
In fact, the libc version is never used in HEAD, stable/9 and stable/8 
and only the libkern version has a small number of clients (see below).

Given that libkern has inet_ntop, with the same arguments of the
userspace version, we'd be much better off with the following
course of action:

1. replace calls to inet_ntoa_r() with inet_ntop()
   This needs to be done only in the kernel, because the libc version
   is never used at least in the source tree (maybe some port does).

2. nuke inet_ntoa_r() from libkern

3. nuke inet_ntoa_r() from libc

The discussion on the cost of the extra argument is not very
relevant here, because the function is intrinsically slow and
called on slow paths (involves an snprintf, and likely some
device I/O downstream).

cheers
luigi


# grep -r   net_ntoa_r . | grep -Ev pristine
./include/arpa/inet.h:#define   inet_ntoa_r __inet_ntoa_r
./include/arpa/inet.h:char  *inet_ntoa_r(struct in_addr, char *buf, 
socklen_t size);
./lib/libc/inet/Symbol.map: __inet_ntoa_r;
./lib/libc/inet/Symbol.map: inet_ntoa_r;
./lib/libc/inet/inet_ntoa.c:inet_ntoa_r(struct in_addr in, char *buf, socklen_t 
size)
./lib/libc/inet/inet_ntoa.c:__weak_reference(__inet_ntoa_r, inet_ntoa_r);
./lib/libc/net/Makefile.inc:inet.3 inet_network.3 inet.3 inet_ntoa.3 inet.3 
inet_ntoa_r.3\
./lib/libc/net/inet.3:.Nm inet_ntoa_r ,
./lib/libc/net/inet.3:.Fo inet_ntoa_r
./lib/libc/net/inet.3:.Fn inet_ntoa_r
./sys/libkern/inet_ntoa.c:inet_ntoa_r(struct in_addr ina, char *buf)
./sys/net/flowtable.c:  inet_ntoa_r(ssin-sin_addr, saddr);
./sys/net/flowtable.c:  inet_ntoa_r(dsin-sin_addr, daddr);
./sys/net/flowtable.c:  inet_ntoa_r(*(struct in_addr *) 
dsin-sin_addr, daddr);
./sys/net/flowtable.c:  inet_ntoa_r(*(struct in_addr *) hashkey[2], daddr);
./sys/net/flowtable.c:  inet_ntoa_r(*(struct in_addr *) hashkey[1], 
saddr);
./sys/net/if_llatbl.c:  inet_ntoa_r(sin-sin_addr, l3s);
./sys/netinet/ipfw/ip_fw_dynamic.c: inet_ntoa_r(da, src);
./sys/netinet/ipfw/ip_fw_dynamic.c: inet_ntoa_r(da, dst);
./sys/netinet/ipfw/ip_fw_dynamic.c: inet_ntoa_r(da, src);
./sys/netinet/ipfw/ip_fw_dynamic.c: inet_ntoa_r(da, dst);
./sys/netinet/ipfw/ip_fw_dynamic.c: inet_ntoa_r(da, src);
./sys/netinet/ipfw/ip_fw_dynamic.c: inet_ntoa_r(da, dst);
./sys/netinet/ipfw/ip_fw_dynamic.c: 
inet_ntoa_r(da, src);
./sys/netinet/ipfw/ip_fw_dynamic.c: 
inet_ntoa_r(da, dst);
./sys/netinet/ipfw/ip_fw_log.c: inet_ntoa_r(ip-ip_src, src);
./sys/netinet/ipfw/ip_fw_log.c: inet_ntoa_r(ip-ip_dst, dst);
./sys/netinet/in.h:char *inet_ntoa_r(struct in_addr ina, char *buf); /* in 
libkern */
./sys/netinet/in_pcb.c: inet_ntoa_r(inc-inc_laddr, laddr_str);
./sys/netinet/in_pcb.c: inet_ntoa_r(inc-inc_faddr, faddr_str);
./sys/netinet/tcp_subr.c:   inet_ntoa_r(inc-inc_faddr, sp);
./sys/netinet/tcp_subr.c:   inet_ntoa_r(inc-inc_laddr, sp);
./sys/netinet/tcp_subr.c:   inet_ntoa_r(ip-ip_src, sp);
./sys/netinet/tcp_subr.c:   inet_ntoa_r(ip-ip_dst, sp);
  
`
I need it because i would like to compile parts of the kernel in userspace,
and having a kernel function with the same name and different arguments
from of a libc function is annoying.
I can accept that

 -- 
 Bjoern A. Zeeb You have to have visions!
  Stop bit received. Insert coin for new address family.
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org


Re: RFC: libkern version of inet_ntoa_r

2012-07-29 Thread Bjoern A. Zeeb

On Sun, 29 Jul 2012, Luigi Rizzo wrote:


On Sat, Jul 28, 2012 at 10:14:10PM +, Bjoern A. Zeeb wrote:

On Wed, 25 Jul 2012, Luigi Rizzo wrote:

..

Given that libkern has inet_ntop, with the same arguments of the
userspace version, we'd be much better off with the following
course of action:

1. replace calls to inet_ntoa_r() with inet_ntop()
  This needs to be done only in the kernel, because the libc version
  is never used at least in the source tree (maybe some port does).

2. nuke inet_ntoa_r() from libkern


I am Ok with that I think (without looking at source implications) but
I guess you'll post a patch.



3. nuke inet_ntoa_r() from libc


I am not sure we can easily do that (despite not being used in base).
I fear some broader research into ports is needed, or as a shortcut,
does linux have an inet_ntoa_r()?


--
Bjoern A. Zeeb You have to have visions!
 Stop bit received. Insert coin for new address family.
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org


Re: RFC: libkern version of inet_ntoa_r

2012-07-29 Thread David Chisnall
On 29 Jul 2012, at 10:58, Luigi Rizzo wrote:

 3. nuke inet_ntoa_r() from libc

inet_ntoa_r is a public symbol and therefore part of our ABI contract with 
userspace applications.  Even if no one that we are aware of is using it, we 
should officially deprecate it for one major release before removing it.  ABI 
churn for purely aesthetic reasons does not make users happy people.  

 I need it because i would like to compile parts of the kernel in userspace,
 and having a kernel function with the same name and different arguments
 from of a libc function is annoying.


Presumably this usage can be trivially fixed with a trivial macro in a prefix 
header?

David___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org


Re: RFC: libkern version of inet_ntoa_r

2012-07-29 Thread Luigi Rizzo
On Sun, Jul 29, 2012 at 05:55:19PM +0100, David Chisnall wrote:
 On 29 Jul 2012, at 10:58, Luigi Rizzo wrote:
 
  3. nuke inet_ntoa_r() from libc
 
 inet_ntoa_r is a public symbol and therefore part of our ABI contract with 
 userspace applications.  Even if no one that we are aware of is using it, we 
 should officially deprecate it for one major release before removing it.  ABI 
 churn for purely aesthetic reasons does not make users happy people.  

sure, interpret nuke as a long term thing
(starting with deprecation, manpage notes, etc.)

 
  I need it because i would like to compile parts of the kernel in userspace,
  and having a kernel function with the same name and different arguments
  from of a libc function is annoying.
 
 
 Presumably this usage can be trivially fixed with a trivial macro in a prefix 
 header?

Remapping f(a) into f(a, b) requires both a macro
and a wrapping function, something like this

T __f(T1 a, T2 b) { return f(a, b); }
#define f(a) __f(a, b)

Surely can be done (in fact, i have done it already, see

 http://info.iet.unipi.it/~luigi/netmap/20120725-ipfw-user.tgz 

but i am not so interested in participating to the IOCCC :)
http://www.ioccc.org/
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org


Re: RFC: libkern version of inet_ntoa_r

2012-07-29 Thread Arnaud Lacombe
Hi,

On Sun, Jul 29, 2012 at 3:19 PM, Luigi Rizzo ri...@iet.unipi.it wrote:
 Remapping f(a) into f(a, b) requires both a macro
 and a wrapping function, something like this

 T __f(T1 a, T2 b) { return f(a, b); }
 #define f(a) __f(a, b)

This can be done way more easily:

void fn(int a, int b)
{
printf(%d %d\n, a, b);
}

#define fn(x)   ({ fn(x, 42); })

int main(int argc, char **argv)
{

fn(0);
return 0;
}

works just fine.

 but i am not so interested in participating to the IOCCC :)

maybe you should ;-)

 - Arnaud

ps: this construct is used all over the Linux kernel compatibility libraries.
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org


Re: RFC: libkern version of inet_ntoa_r

2012-07-29 Thread Luigi Rizzo
On Sun, Jul 29, 2012 at 03:38:59PM -0400, Arnaud Lacombe wrote:
 Hi,
 
 On Sun, Jul 29, 2012 at 3:19 PM, Luigi Rizzo ri...@iet.unipi.it wrote:
  Remapping f(a) into f(a, b) requires both a macro
  and a wrapping function, something like this
 
  T __f(T1 a, T2 b) { return f(a, b); }
  #define f(a) __f(a, b)
 
 This can be done way more easily:
 
 void fn(int a, int b)
 {
 printf(%d %d\n, a, b);
 }
 
 #define fn(x)   ({ fn(x, 42); })

nice trick, one always learns something on these lists...
now i wonder how it works with MSVC (windows being one of the
other platforms where i need to build the ipfw+dummynet code...)

cheers
luigi
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org


Re: RFC: libkern version of inet_ntoa_r

2012-07-29 Thread Lev Serebryakov
Hello, Luigi.
You wrote 30 июля 2012 г., 0:47:21:

 #define fn(x)   ({ fn(x, 42); })
LR nice trick, one always learns something on these lists...
LR now i wonder how it works with MSVC (windows being one of the
LR other platforms where i need to build the ipfw+dummynet code...)
 It looks very gcc-ish.

-- 
// Black Lion AKA Lev Serebryakov l...@freebsd.org

___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org


Re: RFC: libkern version of inet_ntoa_r

2012-07-29 Thread Arnaud Lacombe
Hi,

On Sun, Jul 29, 2012 at 4:30 PM, Lev Serebryakov l...@freebsd.org wrote:
 Hello, Luigi.
 You wrote 30 июля 2012 г., 0:47:21:

 #define fn(x)   ({ fn(x, 42); })
 LR nice trick, one always learns something on these lists...
 LR now i wonder how it works with MSVC (windows being one of the
 LR other platforms where i need to build the ipfw+dummynet code...)
  It looks very gcc-ish.

could you back your point with a technical argument, please ? This
sounds rather FUD'ish so far.

The ({ ... }) is nothing more than a primary-expression enclosing a
compound-statement; this part is not specifically necessary. As for
the expansion, I would assume the following part of iso9899:1999
applies:


6.10.3 Macro replacement

10
A preprocessing directive of the form

# define identifier lparen identifier-listopt ) replacement-list new-line
# define identifier lparen ... ) replacement-list new-line
# define identifier lparen identifier-list , ... ) replacement-list new-line

defines a function-like macro with arguments, similar syntactically to
a function call. [...] Each subsequent instance of the function-like
macro name followed by a ( as the next preprocessing token introduces
the sequence of preprocessing tokens that is replaced by the
replacement list in the definition (an invocation of the macro). The
replaced sequence of preprocessing tokens is terminated by the
matching ) preprocessing token, [...]


Note that the standard say Each subsequent, no All, so

fn(1, 2);
#define fn(a) fn(a, 2)
fn(1);

would produces:

fn(1, 2);

fn(1, 2);

I do not see any gcc'ism here.

Now, I might be wrong, and would enjoy being proven so.

Thanks,
 - Arnaud
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org


Re: RFC: libkern version of inet_ntoa_r

2012-07-28 Thread Bjoern A. Zeeb

On Wed, 25 Jul 2012, Luigi Rizzo wrote:


During some ipfw/dummynet cleanup i noticed that the libkern version of
inet_ntoa_r() is missing the buffer size argument that is present in
the libc counterpart.

Any objection if i fix it ?


And why exactly would you need it?  What does libc do with it?  Render
partial IPv4 addresses?

--
Bjoern A. Zeeb You have to have visions!
 Stop bit received. Insert coin for new address family.
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org


Re: RFC: libkern version of inet_ntoa_r

2012-07-28 Thread Arnaud Lacombe
Hi,

On Sat, Jul 28, 2012 at 6:14 PM, Bjoern A. Zeeb
bzeeb-li...@lists.zabbadoz.net wrote:
 On Wed, 25 Jul 2012, Luigi Rizzo wrote:

 During some ipfw/dummynet cleanup i noticed that the libkern version of
 inet_ntoa_r() is missing the buffer size argument that is present in
 the libc counterpart.

 Any objection if i fix it ?


 And why exactly would you need it?  What does libc do with it?  Render
 partial IPv4 addresses?

Mitigate possibilities of memory corruption ? At the very least, allow
the following:

{
char tmp[sizeof 255.255.255.255];

KASSERT(size = (sizeof tmp));
[...]
}

to be enforced... but hey, who gives a damn about consistently doing
things and enforcing code assumptions ? ;-)

 - Arnaud

 --
 Bjoern A. Zeeb You have to have visions!
  Stop bit received. Insert coin for new address family.

 ___
 freebsd-current@freebsd.org mailing list
 http://lists.freebsd.org/mailman/listinfo/freebsd-current
 To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org


Re: RFC: libkern version of inet_ntoa_r

2012-07-28 Thread Garrett Cooper
On Sat, Jul 28, 2012 at 3:35 PM, Arnaud Lacombe lacom...@gmail.com wrote:
 Hi,

 On Sat, Jul 28, 2012 at 6:14 PM, Bjoern A. Zeeb
 bzeeb-li...@lists.zabbadoz.net wrote:
 On Wed, 25 Jul 2012, Luigi Rizzo wrote:

 During some ipfw/dummynet cleanup i noticed that the libkern version of
 inet_ntoa_r() is missing the buffer size argument that is present in
 the libc counterpart.

 Any objection if i fix it ?


 And why exactly would you need it?  What does libc do with it?  Render
 partial IPv4 addresses?

 Mitigate possibilities of memory corruption ? At the very least, allow
 the following:

 {
 char tmp[sizeof 255.255.255.255];

 KASSERT(size = (sizeof tmp));
 [...]
 }

 to be enforced... but hey, who gives a damn about consistently doing
 things and enforcing code assumptions ? ;-)

I think that a subtlety in Bjoern's reply was missed. Note that
inet_ntoa is guaranteed to only work with IPv4, not IPv4+IPv6, like
inet_ntop.
Thanks,
-Garrett
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org


Re: RFC: libkern version of inet_ntoa_r

2012-07-28 Thread Bjoern A. Zeeb

On Sat, 28 Jul 2012, Arnaud Lacombe wrote:


Hi,

On Sat, Jul 28, 2012 at 6:14 PM, Bjoern A. Zeeb
bzeeb-li...@lists.zabbadoz.net wrote:

On Wed, 25 Jul 2012, Luigi Rizzo wrote:


During some ipfw/dummynet cleanup i noticed that the libkern version of
inet_ntoa_r() is missing the buffer size argument that is present in
the libc counterpart.

Any objection if i fix it ?



And why exactly would you need it?  What does libc do with it?  Render
partial IPv4 addresses?


Mitigate possibilities of memory corruption ? At the very least, allow
the following:

{
   char tmp[sizeof 255.255.255.255];


char tmp[INET_ADDRSTRLEN];



   KASSERT(size = (sizeof tmp));


This would need to go into the called library function and cannot.



   [...]


So that gives you what extra checking exactly?  That the programmer got
the sizeof right rather than the buffer size? You pushed some more on the
stack or reused an register for something that is supposed to be at a
minial fixed length (nothing else lower allowed and will ever result
in anything but misbehaviour) no matter what.  It's not like it's
inet_pton which can take totally different sizes.


Which again leaves me with the question - why does libc have it?

/bz

--
Bjoern A. Zeeb You have to have visions!
 Stop bit received. Insert coin for new address family.
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org


Re: RFC: libkern version of inet_ntoa_r

2012-07-28 Thread Arnaud Lacombe
Hi,

On Sat, Jul 28, 2012 at 6:44 PM, Bjoern A. Zeeb
bzeeb-li...@lists.zabbadoz.net wrote:
 On Sat, 28 Jul 2012, Arnaud Lacombe wrote:

 Hi,

 On Sat, Jul 28, 2012 at 6:14 PM, Bjoern A. Zeeb
 bzeeb-li...@lists.zabbadoz.net wrote:

 On Wed, 25 Jul 2012, Luigi Rizzo wrote:

 During some ipfw/dummynet cleanup i noticed that the libkern version of
 inet_ntoa_r() is missing the buffer size argument that is present in
 the libc counterpart.

 Any objection if i fix it ?



 And why exactly would you need it?  What does libc do with it?  Render
 partial IPv4 addresses?

 Mitigate possibilities of memory corruption ? At the very least, allow
 the following:

 {
char tmp[sizeof 255.255.255.255];
 char tmp[INET_ADDRSTRLEN];

Should I mention that this is taken directly from
`lib/libc/inet/inet_ntop.c' ? you may want to fix it, as you have been
blessed with a commit bit.


KASSERT(size = (sizeof tmp));

 This would need to go into the called library function and cannot.

 So that gives you what extra checking exactly?  That the programmer got
 the sizeof right rather than the buffer size? You pushed some more on the
 stack or reused an register
That is funny. I was told that 2 always unused extra argument all
along the mutex API could not change anything, and now I am told the
opposite for 1 argument. There is no point trading the cost of a
register for overall runtime correctness. This is a string
manipulation function, it must be doing some kind of size check.

 for something that is supposed to be at a  minial fixed length

supposed to be ? you do not seem to be really sure to know how
inet_ntoa_r() is gonna be used, nor have you any way, by your words,
enforce it in any way.

 (nothing else lower allowed and will ever result
 in anything but misbehaviour) no matter what.

I'd be more than happy to welcome you tracking down memory corruption
based misbehaviors, but I prefer to detect it before, than attempt to
catch it after, it happens.

 It's not like it's
 inet_pton which can take totally different sizes.

that's nothing but an implementation details.


 Which again leaves me with the question - why does libc have it?

It is passed down to strlcpy(). You could have found this out by
yourself in a minute or so... But again, implementation details, might
they be incomplete, are irrelevant.

 - Arnaud
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org


Re: RFC: libkern version of inet_ntoa_r

2012-07-28 Thread Arnaud Lacombe
Hi,

On Sat, Jul 28, 2012 at 6:44 PM, Bjoern A. Zeeb
bzeeb-li...@lists.zabbadoz.net wrote:
 Which again leaves me with the question - why does libc have it?

as for the semantic, theoretical, why, I would refer you to the
POSIX's comity, as inet_ntop() is part of it.

 - Arnaud
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org


Re: RFC: libkern version of inet_ntoa_r

2012-07-28 Thread Arnaud Lacombe
Hi,

On Sat, Jul 28, 2012 at 7:46 PM, Arnaud Lacombe lacom...@gmail.com wrote:
 Hi,

 On Sat, Jul 28, 2012 at 6:44 PM, Bjoern A. Zeeb
 bzeeb-li...@lists.zabbadoz.net wrote:
 Which again leaves me with the question - why does libc have it?

 as for the semantic, theoretical, why, I would refer you to the
 POSIX's comity, as inet_ntop() is part of it.

actually, it is slightly more complicated than that. POSIX has
inet_ntoa(), inet_ntop() and no inet_ntoa_r(). libc's inet_ntoa{,_r}()
are implemented on top inet_ntop(), which *does* fail if the provided
buffer is not large enough, and set errno to ENOSPC. However,
inet_ntoa{,_r}() do not propagate inet_ntop() failure.

As for the userland version of inet_ntoa{,_r}(), I would change it to
at least stop ignoring inet_ntop() return value, add an assertion on
its success, drop this awful `strcpy(ret, [inet_ntoa error]);' and
use the proper size in inet_ntoa() definition. As for the kernel
version... it is a lost cause to argue one way or another...

 - Arnaud
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org


RFC: libkern version of inet_ntoa_r

2012-07-25 Thread Luigi Rizzo
During some ipfw/dummynet cleanup i noticed that the libkern version of
inet_ntoa_r() is missing the buffer size argument that is present in
the libc counterpart.

Any objection if i fix it ?
The change is trivial and the function is used only in a small number
of places, see below (some of which are even commented out).

# (cd ~/FreeBSD/head/sys; grep -r inet_ntoa_r .)
./libkern/inet_ntoa.c:inet_ntoa_r(struct in_addr ina, char *buf)
./net/flowtable.c:  inet_ntoa_r(ssin-sin_addr, saddr);
./net/flowtable.c:  inet_ntoa_r(dsin-sin_addr, daddr);
./net/flowtable.c:  inet_ntoa_r(*(struct in_addr *) 
dsin-sin_addr, daddr);
./net/flowtable.c:  inet_ntoa_r(*(struct in_addr *) hashkey[2], daddr);
./net/flowtable.c:  inet_ntoa_r(*(struct in_addr *) hashkey[1], 
saddr);
./net/if_llatbl.c:  inet_ntoa_r(sin-sin_addr, l3s);
./netinet/ipfw/ip_fw_dynamic.c: inet_ntoa_r(da, src);
./netinet/ipfw/ip_fw_dynamic.c: inet_ntoa_r(da, dst);
./netinet/ipfw/ip_fw_dynamic.c: inet_ntoa_r(da, src);
./netinet/ipfw/ip_fw_dynamic.c: inet_ntoa_r(da, dst);
./netinet/ipfw/ip_fw_dynamic.c: inet_ntoa_r(da, src);
./netinet/ipfw/ip_fw_dynamic.c: inet_ntoa_r(da, dst);
./netinet/ipfw/ip_fw_dynamic.c: 
inet_ntoa_r(da, src);
./netinet/ipfw/ip_fw_dynamic.c: 
inet_ntoa_r(da, dst);
./netinet/ipfw/ip_fw_log.c: inet_ntoa_r(ip-ip_src, src);
./netinet/ipfw/ip_fw_log.c: inet_ntoa_r(ip-ip_dst, dst);
./netinet/in.h:char *inet_ntoa_r(struct in_addr ina, char *buf); /* in 
libkern */
./netinet/in_pcb.c: inet_ntoa_r(inc-inc_laddr, laddr_str);
./netinet/in_pcb.c: inet_ntoa_r(inc-inc_faddr, faddr_str);
./netinet/tcp_subr.c:   inet_ntoa_r(inc-inc_faddr, sp);
./netinet/tcp_subr.c:   inet_ntoa_r(inc-inc_laddr, sp);
./netinet/tcp_subr.c:   inet_ntoa_r(ip-ip_src, sp);
./netinet/tcp_subr.c:   inet_ntoa_r(ip-ip_dst, sp);

cheers
luigi
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org