Hi Grant,
On Tue, May 24, 2011 at 11:29:44PM -0700, Grant Erickson wrote:
> Return and pass a consistent set of informative error codes and
> display a consistent set of error messages for
> connman_inet_modify_address and connman_{clear,set}_*_address.
Nice cleanup. I have some comments though:
> @@ -552,11 +555,12 @@ int connman_inet_set_ipv6_address(int index,
>
> DBG("index %d address %s prefix_len %d", index, address, prefix_len);
>
> - if ((__connman_inet_modify_address(RTM_NEWADDR,
> - NLM_F_REPLACE | NLM_F_ACK, index, AF_INET6,
> - address, NULL, prefix_len, NULL)) < 0) {
> - connman_error("Set IPv6 address error");
> - return -1;
> + err = __connman_inet_modify_address(RTM_NEWADDR,
> + NLM_F_REPLACE | NLM_F_ACK, index, AF_INET6,
> + address, NULL, prefix_len, NULL);
> + if (err < 0) {
> + connman_error("Set IPv6 address error: %s", strerror(-err));
I'd prefer: connman_error("%s: %s", __func__, strerror(-err));
> @@ -577,11 +582,12 @@ int connman_inet_set_address(int index, struct
> connman_ipaddress *ipaddress)
>
> DBG("index %d address %s prefix_len %d", index, address, prefix_len);
>
> - if ((__connman_inet_modify_address(RTM_NEWADDR,
> - NLM_F_REPLACE | NLM_F_ACK, index, AF_INET,
> - address, peer, prefix_len, broadcast)) < 0) {
> - DBG("address setting failed");
> - return -1;
> + err = __connman_inet_modify_address(RTM_NEWADDR,
> + NLM_F_REPLACE | NLM_F_ACK, index, AF_INET,
> + address, peer, prefix_len, broadcast);
> + if (err < 0) {
> + connman_debug("Set IPv4 address error: %s", strerror(-err));
DBG() is preferred over connman_debug(), although I wouldn't mind changing
this one into a connman_error()
> @@ -590,12 +596,15 @@ int connman_inet_set_address(int index, struct
> connman_ipaddress *ipaddress)
> int connman_inet_clear_ipv6_address(int index, const char *address,
> int prefix_len)
> {
> + int err;
> +
> DBG("index %d address %s prefix_len %d", index, address, prefix_len);
>
> - if ((__connman_inet_modify_address(RTM_DELADDR, 0, index, AF_INET6,
> - address, NULL, prefix_len, NULL)) < 0) {
> - connman_error("Clear IPv6 address error");
> - return -1;
> + err = __connman_inet_modify_address(RTM_DELADDR, 0, index, AF_INET6,
> + address, NULL, prefix_len, NULL);
> + if (err < 0) {
> + connman_error("Clear IPv6 address error: %s", strerror(-err));
> + return err;
connman_error("%s: %s",....
> int connman_inet_clear_address(int index, struct connman_ipaddress
> *ipaddress)
> {
> + int err;
> unsigned char prefix_len;
> const char *address, *broadcast, *peer;
>
> @@ -613,10 +623,11 @@ int connman_inet_clear_address(int index, struct
> connman_ipaddress *ipaddress)
>
> DBG("index %d address %s prefix_len %d", index, address, prefix_len);
>
> - if ((__connman_inet_modify_address(RTM_DELADDR, 0, index, AF_INET,
> - address, peer, prefix_len, broadcast)) < 0) {
> - DBG("address removal failed");
> - return -1;
> + err = __connman_inet_modify_address(RTM_DELADDR, 0, index, AF_INET,
> + address, peer, prefix_len, broadcast);
> + if (err < 0) {
> + connman_debug("Clear IPv4 address error: %s", strerror(-err));
Same here, s/connman_debug/DBG/, although connman_error() would be more
appropriate.
Cheers,
Samuel.
--
Intel Open Source Technology Centre
http://oss.intel.com/
_______________________________________________
connman mailing list
[email protected]
http://lists.connman.net/listinfo/connman
