Bug 217126 - Completion of error handling
Summary: Completion of error handling
Status: NEW
Alias: None
Product: Tools
Classification: Unclassified
Component: Trace-cmd/Kernelshark (show other bugs)
Hardware: All Linux
: P1 normal
Assignee: Default virtual assignee for Trace-cmd and kernelshark
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2023-03-04 08:19 UTC by Markus Elfring
Modified: 2023-06-02 16:02 UTC (History)
1 user (show)

See Also:
Kernel Version: 6.2.2
Subsystem:
Regression: No
Bisected commit-id:


Attachments

Description Markus Elfring 2023-03-04 08:19:43 UTC
Would you like to add more error handling for return values from functions like the following?

* dirname ⇒ get_source_plugins_dir
  https://git.kernel.org/pub/scm/utils/trace-cmd/trace-cmd.git/tree/lib/trace-cmd/trace-plugin.c?id=0a68daed9f58e4429c0f1e7818f7cc0634873112#n203

* fclose ⇒ dump_file_content
  https://git.kernel.org/pub/scm/utils/trace-cmd/trace-cmd.git/tree/tracecmd/trace-list.c?id=0a68daed9f58e4429c0f1e7818f7cc0634873112#n29

* ftruncate ⇒ update_fd
  https://git.kernel.org/pub/scm/utils/trace-cmd/trace-cmd.git/tree/lib/trace-cmd/trace-recorder.c?id=0a68daed9f58e4429c0f1e7818f7cc0634873112#n306

* strdup ⇒ add_reset_trigger
  https://git.kernel.org/pub/scm/utils/trace-cmd/trace-cmd.git/tree/tracecmd/trace-record.c?id=0a68daed9f58e4429c0f1e7818f7cc0634873112#n257

* printf ⇒ output_event_stack
  https://git.kernel.org/pub/scm/utils/trace-cmd/trace-cmd.git/tree/tracecmd/trace-profile.c?id=0a68daed9f58e4429c0f1e7818f7cc0634873112#n1535

* pthread_mutex_init ⇒ tracecmd_tsync_with_guest
  https://git.kernel.org/pub/scm/utils/trace-cmd/trace-cmd.git/tree/lib/trace-cmd/trace-timesync.c?id=0a68daed9f58e4429c0f1e7818f7cc0634873112#n778

* putchar ⇒ report_probes
  https://git.kernel.org/pub/scm/utils/trace-cmd/trace-cmd.git/tree/tracecmd/trace-stat.c?id=0a68daed9f58e4429c0f1e7818f7cc0634873112#n800

* strdup ⇒ add_reset_trigger
  https://git.kernel.org/pub/scm/utils/trace-cmd/trace-cmd.git/tree/tracecmd/trace-record.c?id=0a68daed9f58e4429c0f1e7818f7cc0634873112#n257

* write ⇒ ptp_clock_server
  https://git.kernel.org/pub/scm/utils/trace-cmd/trace-cmd.git/tree/lib/trace-cmd/trace-timesync-ptp.c?id=0a68daed9f58e4429c0f1e7818f7cc0634873112#n636
Comment 1 Steven Rostedt 2023-03-05 16:58:21 UTC
dirname() is guaranteed to succeed.

fclose() : What error handling would you suggest? Really, if it fails, then there's nothing we can do, and that only happens if there's issues elsewhere. So I ignore it.

ftruncate() errors are all for a bad fd which is covered by the open.

I agree that the strdup()s could use a check.

I don't care about the return value of printf().

The pthread code could use checks.

I don't care about putchar(). If there's an error with printing, how would you report it?

The write in ptp_clock_server is for debugging only. No need to check.

So, I will keep this bug open for the strdup() and pthread code.
Comment 2 Markus Elfring 2023-03-05 20:35:48 UTC
(In reply to Steven Rostedt from comment #1)
More return values should be used (according to rules for secure programming).
* https://wiki.sei.cmu.edu/confluence/display/c/EXP12-C.+Do+not+ignore+values+returned+by+functions
* https://cwe.mitre.org/data/definitions/252.html


How will chances evolve to benefit any more also from the means of aspect-oriented software development?
https://en.wikipedia.org/wiki/Aspect-oriented_programming
Comment 3 Markus Elfring 2023-06-02 16:02:53 UTC
The source code is evolving.

trace-cmd: Check all strdup() return values
https://lore.kernel.org/linux-trace-devel/20230602014539.013dceb2@rorschach.local.home/

Note You need to log in before you can comment on or make changes to this bug.