Change in libosmocore[master]: socket: add osmo_sockaddr_cmp()

[neels]

neels has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/19415 )

Change subject: socket: add osmo_sockaddr_cmp()
......................................................................


Patch Set 12: Code-Review-1

(10 comments)

https://gerrit.osmocom.org/c/libosmocore/+/19415/11/src/socket.c 
File src/socket.c:

https://gerrit.osmocom.org/c/libosmocore/+/19415/11/src/socket.c@1562 
PS11, Line 1562: /*! Compare two osmo_sockaddr. Return 0 if they are same
"the same". End with a ".". Actually the \return is already below.


https://gerrit.osmocom.org/c/libosmocore/+/19415/11/src/socket.c@1563 
PS11, Line 1563:  * \brief osmo_sockaddr_cmp
we use AUTO_BRIEF, so drop this \brief line


https://gerrit.osmocom.org/c/libosmocore/+/19415/11/src/socket.c@1566 
PS11, Line 1566:  * \return 0 if a and b are equal.
the naming of "cmp" implies being able to use the return code for sorting.
So please return 1 if a > b and -1 if a < b.

If you don't want to do that, s/_cmp/_same


https://gerrit.osmocom.org/c/libosmocore/+/19415/11/src/socket.c@1568 
PS11, Line 1568: int osmo_sockaddr_cmp(struct osmo_sockaddr *a, struct osmo_sockaddr *b)
> As usual, I see no point in checking and returning stuff for NULL pointers. […]
I strongly favor generic cmp functions being useful with NULL args as well, it's not much effort and gives a warm and comfortable peace of mind


https://gerrit.osmocom.org/c/libosmocore/+/19415/11/src/socket.c@1570 
PS11, Line 1570: 	if (!a && !b)
this could be just

  if (a == b)
      return 0;


https://gerrit.osmocom.org/c/libosmocore/+/19415/11/src/socket.c@1573 
PS11, Line 1573: 		return 1;
-1


https://gerrit.osmocom.org/c/libosmocore/+/19415/11/src/socket.c@1577 
PS11, Line 1577: 		return 1;
return OSMO_CMP(a->.., b->..)


https://gerrit.osmocom.org/c/libosmocore/+/19415/11/src/socket.c@1583 
PS11, Line 1583: 			 a->u.sin.sin_port == b->u.sin.sin_port);
cleaner, type wise, would be to return <condition> ? 0 : 1

not sure how to do proper cmp rc of -1/1 here... at least the port could use OSMO_CMP()


https://gerrit.osmocom.org/c/libosmocore/+/19415/11/src/socket.c@1585 
PS11, Line 1585: 		/* AF_INET6 usally not contain any padding */
"does not"


https://gerrit.osmocom.org/c/libosmocore/+/19415/11/src/socket.c@1588 
PS11, Line 1588: 		/* fallback to memcmp for remaing AF */
"remaining"



-- 
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/19415
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings

Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I2d12ebae2710ffd17cf071e6ada0804e73f87dd6
Gerrit-Change-Number: 19415
Gerrit-PatchSet: 12
Gerrit-Owner: lynxis lazus <lynxis at fe80.eu>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge at osmocom.org>
Gerrit-Reviewer: lynxis lazus <lynxis at fe80.eu>
Gerrit-Reviewer: neels <nhofmeyr at sysmocom.de>
Gerrit-Reviewer: pespin <pespin at sysmocom.de>
Gerrit-Comment-Date: Thu, 27 Aug 2020 11:14:20 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: pespin <pespin at sysmocom.de>
Gerrit-MessageType: comment
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20200827/dbba66dc/attachment.htm>