Change in libosmocore[master]: add osmo_hexdump_buf() and test

[Vadim Yanitskiy]

Vadim Yanitskiy has posted comments on this change. ( https://gerrit.osmocom.org/12658 )

Change subject: add osmo_hexdump_buf() and test
......................................................................


Patch Set 6:

(4 comments)

https://gerrit.osmocom.org/#/c/12658/6/src/utils.c
File src/utils.c:

https://gerrit.osmocom.org/#/c/12658/6/src/utils.c@230
PS6, Line 230: or "" (empty string) if out_buf == NULL
Any use cases for this tolerance?
Shouldn't we just ASSERT here?


https://gerrit.osmocom.org/#/c/12658/6/src/utils.c@230
PS6, Line 230: out_buf_size < 1
similar question for "out_buf_size < 1".


https://gerrit.osmocom.org/#/c/12658/6/src/utils.c@238
PS6, Line 238: const char *osmo_hexdump_buf(char *out_buf, size_t out_buf_size, const unsigned char *buf, int len, const char *delim,
             : 			     bool delim_after_last)
Some notes:

  - Why do we still use int for the input buffer length?
    Why not size_t as for the output buffer?

  - Are there any use cases when delim is more than one char?

Cosmetic notes:

  - What about prefixing both buffers with 'dst' and 'src'?

  - I would rather format it like this:

    const char *osmo_hexdump_buf(char *out_buf, size_t out_buf_size,
                                 const unsigned char *buf, int len,
                                 const char *delim, bool delim_after_last)

    in order to improve readability.


https://gerrit.osmocom.org/#/c/12658/6/src/utils.c@252
PS6, Line 252: const char *delimp = delim;
Do we really need this pointer? Why not to use delim itself?



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

Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I590595567b218b24e53c9eb1fd8736c0324d371d
Gerrit-Change-Number: 12658
Gerrit-PatchSet: 6
Gerrit-Owner: Neels Hofmeyr <nhofmeyr at sysmocom.de>
Gerrit-Reviewer: Harald Welte <laforge at gnumonks.org>
Gerrit-Reviewer: Jenkins Builder (1000002)
Gerrit-Reviewer: Neels Hofmeyr <nhofmeyr at sysmocom.de>
Gerrit-CC: Vadim Yanitskiy <axilirator at gmail.com>
Gerrit-Comment-Date: Fri, 25 Jan 2019 11:41:29 +0000
Gerrit-HasComments: Yes
Gerrit-HasLabels: No
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20190125/d114485c/attachment.htm>