Max has posted comments on this change. ( https://gerrit.osmocom.org/12017 )

Change subject: msgb: add test helpers
......................................................................


Patch Set 5:

(1 comment)

> I usually end up tailoring the debug output to the specific test.

You can continue doing that. I use those helpers in tests in 2 different 
patches in this series and it seem to work quite fine. I don't intend to 
accommodate to every imaginable use-case.

> when an error occurs, I sometimes needed the position of the difference 
> indicated (usually only during development cycles)

That was main motivation for me as well.

> Could add hex string API (in a later patch?)

I'm not really sure how this should look like so I think it's better to be 
expanded by the person who'd use it that way. It's simply more efficient than 
me trying to guess someone else's use-case and devise API for it.

> and could add difference position indicator (in a later patch?).

It's already there - the position is indicated by !! sign when printing msgb 
and data arrays side-by-side.

> Might be interesting to take a look through other msgb/buffer printing 
> routines flying around in various tests today? Maybe figure out a smaller set 
> of API satisfying all cases?

I'd rather not spend too much time on this. This patch emerged as a simple way 
to aid in testing LCLS-related encoders/decoders where I've got to compare 
rather long arrays and wanted to be able to easily see where they differ.

It would be nice to digg through msgb API but I'm afraid there're more pressing 
tasks ATM.

https://gerrit.osmocom.org/#/c/12017/5/src/msgb.c
File src/msgb.c:

https://gerrit.osmocom.org/#/c/12017/5/src/msgb.c@198
PS5, Line 198: printf
> one slight problem with LOGP could be that in regression tests, we often only 
> check stdout.
That should be fixed by additional patches before this one in latest revision.



--
To view, visit https://gerrit.osmocom.org/12017
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: I3bc95f2f5ab6e3f4b502647fb3e0aaaf1f7c4cf5
Gerrit-Change-Number: 12017
Gerrit-PatchSet: 5
Gerrit-Owner: Max <[email protected]>
Gerrit-Reviewer: Harald Welte <[email protected]>
Gerrit-Reviewer: Jenkins Builder (1000002)
Gerrit-Reviewer: Max <[email protected]>
Gerrit-Reviewer: Neels Hofmeyr <[email protected]>
Gerrit-Reviewer: Pau Espin Pedrol <[email protected]>
Gerrit-Reviewer: Vadim Yanitskiy <[email protected]>
Gerrit-Comment-Date: Tue, 04 Dec 2018 12:01:28 +0000
Gerrit-HasComments: Yes
Gerrit-HasLabels: No

Reply via email to