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

#define not marked as a definition #343

Open
Fomys opened this issue Oct 3, 2024 · 4 comments · May be fixed by #344
Open

#define not marked as a definition #343

Fomys opened this issue Oct 3, 2024 · 4 comments · May be fixed by #344
Labels

Comments

@Fomys
Copy link
Contributor

Fomys commented Oct 3, 2024

Hello,

I believe there is an issue with the #define parsing in the code here. This definition is not being detected when searching for pm_ptr.

Additionally, just above it, pm_sleep_ptr is not even considered as an identifier, so it cannot be found anywhere.

Same for mmSPI_CONFIG_CNTL_1, at least used here and defined here (BTW on the same page you can see many missing links)

It is also the case for older linux version, I just checked with 6.3 and 5.19, pm_ptr and pm_sleep_ptr are not parsed correctly.

@fstachura
Copy link
Collaborator

It seems that pm_sleep and pm_sleep_ptr are filtered out by this line https://github.com/bootlin/elixir/blob/master/script.sh#L169

root@01eac2eb3583:/usr/local/elixir# ctags -x --kinds-c=+p+x --extras='-{anonymous}' pm.h | grep pm_ptr                                 
pm_ptr           macro       475 pm.h             #define pm_ptr(_ptr) PTR_IF(IS_ENABLED(CONFIG_PM), (_ptr))
root@01eac2eb3583:/usr/local/elixir# ctags -x --kinds-c=+p+x --extras='-{anonymous}' pm.h | grep pm_sleep_ptr
pm_sleep_ptr     macro       476 pm.h             #define pm_sleep_ptr(_ptr) PTR_IF(IS_ENABLED(CONFIG_PM_SLEEP), (_ptr))

Notice CONFIG_ in both lines. Same with mmSPI_CONFIG_CNTL_1 and others from that page. All contain CONFIG_.

@tleb
Copy link
Member

tleb commented Oct 4, 2024

One line fix, nice catch @fstachura and @Fomys:

... | grep -avE "^operator |^CONFIG_" | ...

I'll send the fix. Maybe using -e twice if that is supported with the current flags passed.

@fstachura
Copy link
Collaborator

@tleb I have a fix ready, the question is - does filtering identifies that start with CONFIG_ make sense? There are some that are not in Kconfig

@tleb
Copy link
Member

tleb commented Oct 4, 2024

That sounds like a rather unrelated question. Indeed I don't see a reason for filtering those out; they are defines nonetheless that might interest someone someday. Sadly Git log cannot help us: 72571fb.

Attaching the output of the following commands:

⟩ fd -tf | grep -v Kconfig | \
	parallel -L1000 ctags -x 2>/dev/null | \
	grep ^CONFIG_ | sort > /tmp/linux-config-c.txt
⟩ fd -tf | grep -v -e Kconfig -e '\.c$' -e '\.h$' | \
	parallel -L1000 ctags -x 2>/dev/null | \
	grep ^CONFIG_ | sort > /tmp/linux-config-remainder.txt

linux-config-c.txt
linux-config-remainder.txt

For the operator<space> grep, the matches are:

⟩ fd -tf | parallel -L1000 ctags -x 2>/dev/null | grep '^operator '
operator         member      174 kernel/trace/trace_events_hist.c enum field_op_id operator;
operator ->      function     48 tools/testing/selftests/bpf/test_cpp.cpp const T* operator->() const { return skel; }
operator ->      function     50 tools/testing/selftests/bpf/test_cpp.cpp T* operator->() { return skel; }

Let's do that in two steps: first fix the current issue (at least in the code, we don't have to do the full reindex straight away). Then we can discuss about that indexing logic.


edit: related:

elixir/update.py

Lines 318 to 326 in 55921f1

if (db.defs.exists(tok) and
not ( (idx*idx_key_mod + line_num) in defs_idxes and
defs_idxes[idx*idx_key_mod + line_num] == tok ) and
(family != 'M' or tok.startswith(b'CONFIG_'))):
# We only index CONFIG_??? in makefiles
if tok in idents:
idents[tok] += ',' + str(line_num)
else:
idents[tok] = str(line_num)

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

Successfully merging a pull request may close this issue.

3 participants