Hi Dan,
On 1/18/2022 3:38 AM, Dan Carpenter wrote:
[ This is an older warning but renaming the function made it appear in
the new warnings list. I have searched my outbox and I don't think
I forwarded this one before. -dan ]
Hello Tom Zanussi,
The patch ec5ce0987541: "tracing: Allow whitespace to surround hist
trigger filter" from Jan 15, 2018, leads to the following Smatch
static checker warning:
kernel/trace/trace_events_hist.c:6199 event_hist_trigger_parse()
warn: 'p' can't be NULL.
kernel/trace/trace_events_hist.c
6149 static int event_hist_trigger_parse(struct event_command *cmd_ops,
6150 struct trace_event_file *file,
6151 char *glob, char *cmd, char *param)
6152 {
6153 unsigned int hist_trigger_bits = TRACING_MAP_BITS_DEFAULT;
6154 struct event_trigger_data *trigger_data;
6155 struct hist_trigger_attrs *attrs;
6156 struct event_trigger_ops *trigger_ops;
6157 struct hist_trigger_data *hist_data;
6158 struct synth_event *se;
6159 const char *se_name;
6160 bool remove = false;
6161 char *trigger, *p, *start;
6162 int ret = 0;
6163
6164 lockdep_assert_held(&event_mutex);
6165
6166 if (glob && strlen(glob)) {
6167 hist_err_clear();
6168 last_cmd_set(file, param);
6169 }
6170
6171 if (!param)
6172 return -EINVAL;
6173
6174 if (glob[0] == '!')
6175 remove = true;
6176
6177 /*
6178 * separate the trigger from the filter (k:v [if filter])
6179 * allowing for whitespace in the trigger
6180 */
6181 p = trigger = param;
6182 do {
6183 p = strstr(p, "if");
6184 if (!p)
6185 break;
6186 if (p == param)
6187 return -EINVAL;
6188 if (*(p - 1) != ' ' && *(p - 1) != '\t') {
6189 p++;
6190 continue;
^^^^^^^^^
These are the continue paths
6191 }
6192 if (p >= param + strlen(param) - (sizeof("if") - 1) - 1)
6193 return -EINVAL;
6194 if (*(p + sizeof("if") - 1) != ' ' && *(p + sizeof("if") - 1) != '\t') {
6195 p++;
6196 continue;
^^^^^^^^^
6197 }
6198 break;
--> 6199 } while (p);
Should this be } while(*p);?
No, it shouldn't be *p, it should be p, but the check at the top of the loop (if (!p) break) makes it redundant. I'll submit a patch to change it to while (1) to avoid the warning.
Thanks,
Tom
6200
6201 if (!p)
6202 param = NULL;
6203 else {
6204 *(p - 1) = '\0';
6205 param = strstrip(p);
6206 trigger = strstrip(trigger);
6207 }
6208
regards,
dan carpenter
