fix(plugin): make ida_compat.hpp self-contained (include <moves.hpp>)#38
Merged
Conversation
The bookmarks shim references bookmarks_t/lochist_entry_t/idaplace_t/tiplace_t
from <moves.hpp> but didn't include it, relying on ida_headers.hpp pulling
moves.hpp in first. The plugin's main.cpp includes ida_compat.hpp directly with
a curated header set that omits moves.hpp, so the idasql_plugin target failed to
compile ('use of undeclared identifier bookmarks_t', ida_compat.hpp:116) on IDA
SDK 9.3 while idasql_cli (via ida_headers.hpp) built fine. Include <moves.hpp>
in the shim so it no longer depends on include ordering. (#35)
0xeb
added a commit
that referenced
this pull request
Jun 23, 2026
… shim bookmarks_t::get_by_inode() is 9.3-only; #35 emulated it on pre-9.3 SDKs by scanning the store. But the collectors only ever need inode -> slot, and the standard bookmark dirtrees key each leaf inode by the place's primary coordinate (idaplace -> ea, tiplace -> ordinal). So invert the lookup: enumerate the store with the stable size()/get() API and join folder info from the dirtree inode->path map by coordinate. No get_by_inode call, no version gate -- one identical code path on every SDK (9.0-9.3). - symbols_bookmarks: now store-driven (was dirtree-driven); also stops silently dropping idaplace bookmarks that lack a dirtree leaf (kNoLeafInode sentinel). - types_bookmarks: drop the get_by_inode overlay loop; key inode_paths by ordinal. - ida_compat.hpp: remove idasql_bookmarks_get_by_inode, the IDASQL_HAS_BOOKMARKS_GET_BY_INODE macro, and the now-unneeded <moves.hpp> include (added in #38 solely for that shim). Verified on 9.3 (CLI + HTTP): bookmarks/local_type_bookmarks insert, folder set, description update, delete; inode resolves to ea/ordinal exactly as get_by_inode.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pre-existing build break (IDA SDK 9.3)
idasql_pluginfailed to compile:idasql_clibuilt fine, which is the tell.Root cause
ida_compat.hpp's bookmarks shim (idasql_bookmarks_get_by_inode) usesbookmarks_t/lochist_entry_t/idaplace_t/tiplace_tfrom<moves.hpp>, but doesn't include it — it relied onida_headers.hppincludingmoves.hpp(line 80) beforeida_compat.hpp(line 102). The plugin (src/plugin/main.cpp:77) includesida_compat.hppdirectly with a curated header set (ida/idp/loader/kernwin/auto) that omitsmoves.hpp, so the type was undeclared. The CLI goes throughida_headers.hpp, so it never hit it.Fix
#include <moves.hpp>at the top ofida_compat.hpp— the shim now includes what it uses and no longer depends on include ordering. Include guards make this free for theida_headers.hpppath.Verified
Full build clean:
xsql,idasql,idasql_cli,idasql_pluginall built (IDA SDK 9.30). Unrelated to but discovered during #37.