-
Notifications
You must be signed in to change notification settings - Fork 91
Potential fix for code scanning alert no. 316: Incomplete ordering #1284
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
Conversation
1c93cf5 to
73741e9
Compare
...eQL) Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
73741e9 to
047d880
Compare
It looks like the ordering methods of YAFFSEntry is unused, but tricks other tools into proposing even more complete unused ordering implementation.
e3krisztian
commented
Nov 17, 2025
I was looking at the code and if comparison is used at all, but could not find any trivial places (explicit use of < and >, sorted, max, min) that do not use a key parameter for comparisons.
Also checked with CC, which also found no use
●くろまる Based on my analysis, I can see that YAFFSEntry comparison is NOT directly used in the current code. The @functools.total_ordering decorator was added as part of the CodeQL fix, but looking at the code:
- No direct sorting of
YAFFSEntryobjects: The code sorts data_chunks (which areYAFFSChunkobjects, notYAFFSEntryobjects) on lines 597 and 680. - Tree operations use string IDs: The file entries tree uses
str(entry.object_id)as node identifiers (lines 439, 443-444, 458-460, 467, 490-491), not the entry objects themselves for comparison. - The comparison methods exist but appear unused: The
__lt__,__gt__,__eq__methods defined on lines 209-216 compare based on object_id, but they don't seem to be actively invoked by the current code.
Why was @functools.total_ordering added?
Looking at the git commit message, CodeQL reported an "incomplete ordering" issue. This is a defensive fix - even though the comparison isn't currently used, the class defines some comparison methods (__lt__, __gt__, __eq__), which creates an incomplete ordering. The @functools.total_ordering decorator automatically generates the missing comparison methods (__le__, __ge__, __ne__) based on the ones defined, ensuring complete ordering semantics.
Summary:
The YAFFSEntry comparison methods (python/unblob/handlers/filesystem/yaffs.py:209-216) define ordering by object_id, but this ordering doesn't appear to be actively used in the current codebase. The @functools.total_ordering decorator was added as a defensive measure to ensure complete ordering semantics, preventing potential issues if the class is ever used in sorting operations in the future.
So instead of moving forward to total_ordering I propose to simplify the code and remove all the ordering methods.
Potential fix for https://github.com/onekey-sec/unblob/security/code-scanning/316
The best way to ensure complete ordering is to use the
functools.total_orderingdecorator on theYAFFSEntryclass. This decorator automatically fills in the missing rich comparison methods if at least__eq__and one of__lt__,__le__,__gt__, or__ge__are defined. SinceYAFFSEntryalready defines__eq__,__lt__, and__gt__, it's safe to decorate it with@functools.total_ordering. This change should be made directly above the class definition, after importingfunctoolsin the file if it’s not already imported.Suggested fixes powered by Copilot Autofix. Review carefully before merging.