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

Stop using atoi()/atol()/strtol(), convert all to str_to_*() and str_to_*_strict() #966

Open
biergaizi opened this issue Jan 25, 2021 · 1 comment
Labels
C-str Issues and PRs about C/C++ methods, headers and data types dealing with strings and memory blocks refactor/fightwarn PR or issue proposal to improve code maintainability without functional changes, or to fix warnings

Comments

@biergaizi
Copy link
Contributor

biergaizi commented Jan 25, 2021

In the C programming language, atoi() and atol() don't check errors. If the input string is invalid, their behavior is undefined with no guaranteed return value (a particular implementation of libc may have defined return value, but it's not guaranteed by the C standard). strtol() is better, but its error checking is complicated, one need to test three conditions, errno != 0 for overflows and invalid integers, nptr != endptr for strings without digits, and *endptr != '\0' for strings mixed with letters and numbers, it is easily misused. Also, it ignores the leading space in the string that can hide bugs. *BSD's strtonum() is the best function for this job, but it's not portable.

To solve this problem, the NUT project already has a builtin library common/str.c that provides easy-to-use and portable string conversion functions with robust error handling, such as str_to_short(), str_to_ushort(), str_to_long() (they are implemented by calling strtol() with correct error checking), "strict" versions that report errors if there are leading spaces also exist. Unfortunately, these good string functions is used by almost nobody. They are only used by 3 drivers, and it's not used in server code at all.

$ grep -R str_to *.c *.h | uniq 
nutdrv_siemens_sitop.c:		if (str_to_uint(maxPollsString, &parsed, 10) == 1) {
snmp-ups.c:			if ( !str_to_long(val ? val : su_info_p->dfl, &value, 10) ) {
snmp-ups.c:				if ( !str_to_long(val, &value, 10) ) {

$ grep -R str_to *.c *.h | wc -l
0

All atoi(), atol(), strtol() in the existing code should be converted to use str_to_*() instead, if it's not practical to convert existing code, at least they should be disallowed in new code. Ideally, the "strict" version of the functions should be preferred.

The new-drivers.txt document should inform the developers about the existence of these functions.

@biergaizi
Copy link
Contributor Author

biergaizi commented Jan 25, 2021

Related #676 #677.

@jimklimov jimklimov added the refactor/fightwarn PR or issue proposal to improve code maintainability without functional changes, or to fix warnings label Nov 14, 2021
@jimklimov jimklimov added the C-str Issues and PRs about C/C++ methods, headers and data types dealing with strings and memory blocks label Apr 10, 2024
jimklimov added a commit to jimklimov/nut that referenced this issue Jul 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-str Issues and PRs about C/C++ methods, headers and data types dealing with strings and memory blocks refactor/fightwarn PR or issue proposal to improve code maintainability without functional changes, or to fix warnings
Projects
None yet
Development

No branches or pull requests

2 participants