Change in osmo-hlr[master]: gsupclient: add osmo_gsup_msg_enc_send() helper

[Neels Hofmeyr]

Neels Hofmeyr has posted comments on this change. ( https://gerrit.osmocom.org/11989 )

Change subject: gsupclient: add osmo_gsup_msg_enc_send() helper
......................................................................


Patch Set 2: Code-Review-1

(6 comments)

https://gerrit.osmocom.org/#/c/11989/2//COMMIT_MSG
Commit Message:

https://gerrit.osmocom.org/#/c/11989/2//COMMIT_MSG@7
PS2, Line 7: gsupclient: add osmo_gsup_msg_enc_send() helper
("helper" triggers a rant from me: it is a word like "framework" or "thing". All functions except main() are helpers.)


https://gerrit.osmocom.org/#/c/11989/2//COMMIT_MSG@10
PS2, Line 10: messages using a given abstract 'osmo_gsup_message' structure.
what the function does should be documented at the new function. the commit log is more about understanding why and how the change came to be.

And that is exactly what is missing here. All that I can tell is that you are adding dead code??


https://gerrit.osmocom.org/#/c/11989/2/src/gsupclient/gsup_client.c
File src/gsupclient/gsup_client.c:

https://gerrit.osmocom.org/#/c/11989/2/src/gsupclient/gsup_client.c@342
PS2, Line 342: /* Helper for encoding and sending GSUP messages */
/* Encode and send a GSUP message. */

Though, that part is rather obvious from the function name, while e.g. the return value is not explained.


https://gerrit.osmocom.org/#/c/11989/2/src/gsupclient/gsup_client.c@349
PS2, Line 349: 	/* Allocate GSUP message buffer */
omit comments that merely restate the code


https://gerrit.osmocom.org/#/c/11989/2/src/gsupclient/gsup_client.c@356
PS2, Line 356: 	/* Encode GSUP message */
ditto


https://gerrit.osmocom.org/#/c/11989/2/src/gsupclient/gsup_client.c@363
PS2, Line 363: 	/* Finally send */
Do you know these API docs that go like:

  set_value():             Helper to set value.
  osmo_gsup_encode():      Encode an OsmoGSUP.
  osmo_gsup_client_send(): Send an OsmoGSUP client.
  
;)



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

Gerrit-Project: osmo-hlr
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0589ff27933e9bca2bcf93b8259004935778db8f
Gerrit-Change-Number: 11989
Gerrit-PatchSet: 2
Gerrit-Owner: Vadim Yanitskiy <axilirator at gmail.com>
Gerrit-Reviewer: Harald Welte <laforge at gnumonks.org>
Gerrit-Reviewer: Jenkins Builder (1000002)
Gerrit-Reviewer: Neels Hofmeyr <nhofmeyr at sysmocom.de>
Gerrit-Comment-Date: Thu, 06 Dec 2018 01:47:41 +0000
Gerrit-HasComments: Yes
Gerrit-HasLabels: Yes
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20181206/e43fffe2/attachment.htm>