libubox: Drop extraneous space when appending values to variable

[pprindeville]

If you (as someone extending this library) use __jshn_append() it will create a string with an undesired leading space.

fixes: issue #15

[pprindeville]

Can we please get some progress on this?

[pprindeville]

@hauke @nbd168

[BKPepe]

Robert (@robimarko) do you think thank you could help Philip to look at this? :)

[l-jonas]

Could you describe a "test case" where this helps. This could be just a few lines of codes where the patch creates a difference. Just by looking at it, I don't understand it.

[pprindeville]

Could you describe a "test case" where this helps. This could be just a few lines of codes where the patch creates a difference. Just by looking at it, I don't understand it.

If you do:

_jshn_append var 'foo'
_jshn_append var 'bar'

You'll end up with var=" foo bar" note the leading space.

[l-jonas]

Functions starting with an underscore are internal functions. Are there paths to trigger the difference without using those that affect the final JSON?

[pprindeville]

Functions starting with an underscore are internal functions. Are there paths to trigger the difference without using those that affect the final JSON?

I don't understand the question. json_add_array and json_add_object call _json_add_table which calls _json_add_generic which calls _jshn_append.

One of the variables modified by _json_add_generic is K_${cur} which I use in my proposed changes (PR #6) when I try to get the root position or the index of the cursor inside an array or object.

[pprindeville]

This is trivial to demonstrate. And fix. It's a 3-line test and a 1 line fix.

root@OpenWrt:~# cat test.sh
#!/bin/sh

. /usr/share/libubox/jshn.sh

_jshn_append var 'foo'
echo "var='$var'"

root@OpenWrt:~# sh test.sh
var=' foo'
root@OpenWrt:~# 

This might be why we're not seeing new contributors sign up....

[l-jonas]

If it's trivial to demonstrate, then please demonstrate it. I am not asking for direct calls to this particular function as this is an internal function that should not be used directly. This "names that start with underscore are internal API only" is a common convention used in languages that have no or a limited concept of encapsulation.

So I am asking for a script that constructs a json object, json array or whatever using the public API only (functions not starting with an underscore) where you suggested change causes a difference.

This might be why we're not seeing new contributors sign up....

When contributing, I expect some quality assurance. If I can not explain why my change is an improvement, I don't expect anyone to accept it upstream. That's why the explanation of the change sometimes takes much more effort than the actual change. I don't know your expectations. This is also the other way around: If I see some project merging random stuff without reason, I can not trust that project anymore.

[pprindeville]

If it's trivial to demonstrate, then please demonstrate it. I am not asking for direct calls to this particular function as this is an internal function that should not be used directly. This "names that start with underscore are internal API only" is a common convention used in languages that have no or a limited concept of encapsulation.

So I am asking for a script that constructs a json object, json array or whatever using the public API only (functions not starting with an underscore) where you suggested change causes a difference.

I'm not saying it's publicly callable. But in writing additional functions (which are) that call through it, those functions didn't operate correctly because of the leading space interfering. This fix is a prerequisite to PR #6.

To reiterate, this bug was confirmed by @nbd168 and he provided the fix.

[l-jonas]

So #6 is affected by this changes? Which difference does it create there and where/why?

[pprindeville]

So #6 is affected by this changes? Which difference does it create there and where/why?

I had to copy and paste jshn_get_index into the DHCPv4 importer to get it to work with the packaged libubox.