nvme-print-json: extern json object add functions

[ikegami-t]

Some additinal functions introduced and fix obj_add_uint_0nx error.

[ikegami-t]

Separated the commit from the PR #2484.

[calebsander]

This is a quadratic algorithm for what should be linear. Can you just keep track of the current length in a local variable?

Reversing the bytes also seems a little weird, but I guess I don't know what the intended use case is.

[ikegami-t]

This is a quadratic algorithm for what should be linear. Can you just keep track of the current length in a local variable?

Just fix this as mentioned.

Reversing the bytes also seems a little weird, but I guess I don't know what the intended use case is.

Unfortunately I do not any idea to resovle this. This outputs the little endian bytes endian as a big endian string as below for example.

buf[0] = 0xaa;
buf[1] = 0xbb;
buf[2] = 0xcc;
buf[3] = 0xdd;
value = "0xddccbbaa"

Thank you for your reviewing.

[igaw]

Not really looked into it, so just a stupid comment from my side on the output order. Shouldn't this be output how the spec defines it? IIRC everything as little endian? (if this statement is untrue, I blame my vacation...)

[calebsander]

Agree with @igaw. It's hard to say without knowing how this function is going to be used. (I would expect the functions to be added in the same patch set as users of the functions.) But for generic "print bytes" functionality, it seems like it would make sense to print the bytes in the order they appear. If the function is interpreting the bytes as an integer, it might make more sense to have "unit" in the function name.

[ikegami-t]

Hi,
For example the existing fw-activate-history command outputs the loge page GUID value as below as a big endian string.

tokunori@tokunori-desktop:~/nvme-cli$ nvme-build ocp fw-activate-history /dev/nvme0
Firmware History Log:
  log identifier:           194
  valid entries:            0
  entries:
  log page version:         1
  log page guid:            0xd11cf3ac8ab24de2a3f6dab4769a796d

Also the OCP specification is also describes same as below as D11CF3AC8AB24DE2A3F6DAB4769A796Dh.

So seems the ocp commands just follow the specification descritions.

[calebsander]

Is it necessary to handle all these cases? I don't see the other JSON value add functions checking their arguments like this. Seems like this would be papering over a bug in the caller...

[ikegami-t]

Which do you suggest?

1. Ignore the invalid value with the checking then the function caused a null pointer exception or any unexpected error. (I do not think this function behavior correct.)
2. Change the void function to return any error by the checking?

[ikegami-t]

Fixed this as add a error message by the checking.

[calebsander]

Logging the error at least is an improvement. But in what situation do we expect a caller to pass in these invalid values? Seems like unnecessary overhead to check them. If null pointers are passed, we will get a pretty clear segfault.

[ikegami-t]

Right. Just fixed as below. Thank you.

	if (!buf || !len) {
		obj_add_str(o, k, "No information provided");
		return;
	}

[calebsander]

Isn't this being overwritten by the hex characters of the first byte?

[ikegami-t]

Right. Sorry the fix was not correct. Thanks for your suggestion.

[ikegami-t]

Fixed the overwritten error.

[igaw]

Thanks!