Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

util/json: Use json_object_get_uint64() with uint64 support #259

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

justinrernst
Copy link
Contributor

@justinrernst justinrernst commented Jan 2, 2024

If HAVE_JSON_U64=1, utils/json.c:display_hex() can call json_object_get_int64() on a struct json_object created with json_object_new_uint64(). In the context of 'ndctl list --regions --human', this results in a static value of 0x7fffffffffffffff being displayed for iset_id, as seen in #217.

Correct hex values are observed with the use of json_object_get_uint64(). To support builds against older json-c, use a new static inline function util_json_get_u64() to fallback to json_object_get_int64() if HAVE_JSON_U64=0.

Link: #217
Fixes: 691cd24 ("json: Add support for json_object_new_uint64()")

We observed the 0x7fffffffffffffff value while testing NVDIMMS with RHEL9.2 and RHEL9.3, ndctl version 71.1

# cat /etc/os-release
NAME="Red Hat Enterprise Linux"
VERSION="9.3 (Plow)"
ID="rhel"
ID_LIKE="fedora"
VERSION_ID="9.3"
PLATFORM_ID="platform:el9"
PRETTY_NAME="Red Hat Enterprise Linux 9.3 (Plow)"
ANSI_COLOR="0;31"
LOGO="fedora-logo-icon"
CPE_NAME="cpe:/o:redhat:enterprise_linux:9::baseos"
HOME_URL="https://www.redhat.com/"
DOCUMENTATION_URL="https://access.redhat.com/documentation/en-us/red_hat_enterprise_linux/9"
BUG_REPORT_URL="https://bugzilla.redhat.com/"

REDHAT_BUGZILLA_PRODUCT="Red Hat Enterprise Linux 9"
REDHAT_BUGZILLA_PRODUCT_VERSION=9.3
REDHAT_SUPPORT_PRODUCT="Red Hat Enterprise Linux"
REDHAT_SUPPORT_PRODUCT_VERSION="9.3"
# ndctl --version
71.1
# ndctl list -Ru
[
  {
    "dev":"region1",
    "size":"1518.00 GiB (1629.94 GB)",
    "align":"96.00 MiB (100.66 MB)",
    "available_size":"1518.00 GiB (1629.94 GB)",
    "max_available_extent":"1518.00 GiB (1629.94 GB)",
    "type":"pmem",
    "iset_id":"0x7fffffffffffffff",
    "persistence_domain":"memory_controller"
  },
  {
    "dev":"region3",
    "size":"1518.00 GiB (1629.94 GB)",
    "align":"96.00 MiB (100.66 MB)",
    "available_size":"1518.00 GiB (1629.94 GB)",
    "max_available_extent":"1518.00 GiB (1629.94 GB)",
    "type":"pmem",
    "iset_id":"0x7fffffffffffffff",
    "persistence_domain":"memory_controller"
  },
  {
    "dev":"region0",
    "size":"1518.00 GiB (1629.94 GB)",
    "align":"96.00 MiB (100.66 MB)",
    "available_size":"1518.00 GiB (1629.94 GB)",
    "max_available_extent":"1518.00 GiB (1629.94 GB)",
    "type":"pmem",
    "iset_id":"0x7fffffffffffffff",
    "persistence_domain":"memory_controller"
  },
  {
    "dev":"region2",
    "size":"1518.00 GiB (1629.94 GB)",
    "align":"96.00 MiB (100.66 MB)",
    "available_size":"1518.00 GiB (1629.94 GB)",
    "max_available_extent":"1518.00 GiB (1629.94 GB)",
    "type":"pmem",
    "iset_id":"0x7fffffffffffffff",
    "persistence_domain":"memory_controller"
  }
]

If HAVE_JSON_U64=1, utils/json.c:display_hex() can call json_object_get_int64()
on a struct json_object created with json_object_new_uint64(). In the context of
'ndctl list --regions --human', this results in a static value of 0x7fffffffffffffff
being displayed for iset_id, as seen in pmem#217.

Correct hex values are observed with the use of json_object_get_uint64(). To support
builds against older json-c, use a new static inline function util_json_get_u64() to
fallback to json_object_get_int64() if HAVE_JSON_U64=0.

Link: pmem#217
Fixes: 691cd24 ("json: Add support for json_object_new_uint64()")
Signed-off-by: Justin Ernst <[email protected]>
@stellarhopper
Copy link
Member

@justinrernst This looks good but would you mind sending this as a patch to the mailing list with a proper signoff etc.? (See https://github.com/pmem/ndctl/blob/main/CONTRIBUTING.md )

@justinrernst
Copy link
Contributor Author

@stellarhopper No problem. Sent!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants