Skip to content

Fix processing of bits type#364

Merged
JoseIgnacioTamayo merged 1 commit intorobshakir:masterfrom
dthiele:fix-allowed_bits-0
Oct 13, 2025
Merged

Fix processing of bits type#364
JoseIgnacioTamayo merged 1 commit intorobshakir:masterfrom
dthiele:fix-allowed_bits-0

Conversation

@dthiele
Copy link
Copy Markdown
Contributor

@dthiele dthiele commented Sep 30, 2025

Hi,
I experienced an issue when processing a bits type that contains bit statements without any position statements. The following error message is triggeed by the position-less bit statements in the access-operations-type here https://github.com/mbj4668/pyang/blob/efae923cff35eeb0b9f257903e08c5f6c9de7256/modules/ietf/ietf-netconf-acm.yang#L119

export PYBINDPLUGIN=$(/usr/bin/env python -c 'import pyangbind; import os; print ("{}/plugin".format(os.path.dirname(pyangbind.__file__)))')
pyang -V --plugindir $PYBINDPLUGIN -f pybind -o bindings.py /home/dthiele/repositories/pyangbind/.venv/share/yang/modules/ietf/ietf-system.yang
# module search path: .:/home/dthiele/repositories/pyangbind_orig/.venv/share/yang/modules
# read /home/dthiele/repositories/pyangbind/.venv/share/yang/modules/ietf/ietf-system.yang (CL)
# read /home/dthiele/repositories/pyangbind_orig/.venv/share/yang/modules/ietf/ietf-yang-types.yang
# read /home/dthiele/repositories/pyangbind_orig/.venv/share/yang/modules/ietf/ietf-inet-types.yang
# read /home/dthiele/repositories/pyangbind_orig/.venv/share/yang/modules/ietf/ietf-netconf-acm.yang
# read /home/dthiele/repositories/pyangbind_orig/.venv/share/yang/modules/iana/iana-crypt-hash.yang
# read /home/dthiele/repositories/pyangbind_orig/.venv/share/yang/modules/ietf/ietf-system.yang
Traceback (most recent call last):
  File "/home/dthiele/repositories/pyangbind_orig/.venv/bin/pyang", line 7, in <module>
    sys.exit(run())
             ~~~^^
  File "/home/dthiele/repositories/pyangbind_orig/.venv/lib/python3.13/site-packages/pyang/scripts/pyang_tool.py", line 548, in run
    emit_obj.emit(ctx, modules, fd)
    ~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^
  File "/home/dthiele/repositories/pyangbind_orig/pyangbind/plugin/pybind.py", line 203, in emit
    build_pybind(ctx, modules, fd)
    ~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^
  File "/home/dthiele/repositories/pyangbind_orig/pyangbind/plugin/pybind.py", line 400, in build_pybind
    build_typedefs(ctx, defn["typedef"])
    ~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^
  File "/home/dthiele/repositories/pyangbind_orig/pyangbind/plugin/pybind.py", line 569, in build_typedefs
    cls, elemtype = copy.deepcopy(build_elemtype(ctx, item.search_one("type")))
                                  ~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/dthiele/repositories/pyangbind_orig/pyangbind/plugin/pybind.py", line 1362, in build_elemtype
    pos = 1 + max(allowed_bits)
              ~~~^^^^^^^^^^^^^^
ValueError: max() iterable argument is empty

This PR should fix the issue and adds a corresponding test. In particular:

  • Add a default of -1 to max(allowed_bits), to cover cases where allowed_bits is empty (e.g. in the absence of any position statements). This will yield an initial implicit pos of 0.
  • allowed_bits is a dict. Take the maximum of its values.
  • Raise ValueError, if pos exceeds its bounds.
  • Add test case.

@google-cla
Copy link
Copy Markdown

google-cla bot commented Sep 30, 2025

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@google-cla
Copy link
Copy Markdown

google-cla bot commented Sep 30, 2025

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@JoseIgnacioTamayo
Copy link
Copy Markdown
Collaborator

Thanks for the PR!

For me, LGTM. Still I would appreciate additional reviews from other contributors.

@xavier-contreras
Copy link
Copy Markdown
Collaborator

This looks fine considering

[9.7.4.2](https://datatracker.ietf.org/doc/html/rfc7950#section-9.7.4.2).  The "position" Statement

   The "position" statement, which is optional, takes as an argument a
   non-negative integer value that specifies the bit's position within a
   hypothetical bit field.  The position value MUST be in the range 0 to
   4294967295, and it MUST be unique within the bits type.

   If a bit position is not specified, then one will be automatically
   assigned.

@JoseIgnacioTamayo
Copy link
Copy Markdown
Collaborator

Hi @dthiele

Could you please rebase now, the CI tests are fixed so this MR should look all green

* Add a default of -1 to max(allowed_bits), to cover cases where allowed_bits is empty (e.g. in the absence of any position statements). This will yield an initial implicit position of 0.
* allowed_bits is a dict. Take the maximum of its values.
* Raise ValueError, if pos exceeds its bounds.
* Add test case.
@dthiele dthiele force-pushed the fix-allowed_bits-0 branch from be04bc6 to 7705585 Compare October 12, 2025 23:36
@JoseIgnacioTamayo JoseIgnacioTamayo self-requested a review October 13, 2025 20:30
Copy link
Copy Markdown
Collaborator

@JoseIgnacioTamayo JoseIgnacioTamayo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks for the fix.

@JoseIgnacioTamayo JoseIgnacioTamayo merged commit 2099976 into robshakir:master Oct 13, 2025
8 checks passed
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.

3 participants