On Thu, 25 Feb 2021 15:07:50 +0800 Jacob Wen <jian.w.wen@xxxxxxxxxx> wrote: > > I wonder if we can add something to checkpatch that can check if > > TP_printk() has a call to "%s" where it references a __entry->xxx and > > not a __get_str(), and will warn about it. > That's helpful for me who don't know "%s" of TP_printk is special. Here's nothing special about %s in TP_printk. It uses the same code as printk() and what other string formatters use. What is special is that the print is on data that is stored from a previous time. TP_fast_assign() / TP_printk() is basically this: struct entry { char *name; } entry; TP_fast_assign() { entry.name = slab->name; } TP_printk() { printk("%s", entry.name); } Where TP_printk() can be called some time in the future when a user asks for it. If the slab->name is freed, then the entry.name will be pointing to stale data, and you don't want to call printk() on that! Thus, the "%s" in TP_printk() is nothing special, it's the fact that the data it reads is called much later in time from when that data was recorded. Which means, you can not rely on any dereferencing of pointers. The __string() __assign_str() and __get_str() macros are helpers to easily store strings in the ring buffer, as that is a common practice in the trace events. -- Steve > > > > There a a few cases where its OK. Like RCU uses a TPS() macro around > > strings it passes into the tracepoint, which is used for strings that > > never are freed, and maps the string pointer to the string for user > > space. But RCU is the only user of that I believe. > >