Page MenuHomeFreeBSD

expand_number: Tighten check of unit.
ClosedPublic

Authored by delphij on Jun 10 2023, 4:34 AM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, May 22, 4:28 AM
Unknown Object (File)
Sun, May 19, 1:38 PM
Unknown Object (File)
Sun, May 19, 12:53 PM
Unknown Object (File)
Sun, May 19, 12:53 PM
Unknown Object (File)
Sun, May 19, 12:53 PM
Unknown Object (File)
Sun, May 19, 8:43 AM
Unknown Object (File)
Tue, Apr 30, 9:10 AM
Unknown Object (File)
Mar 12 2024, 3:37 PM
Subscribers

Details

Summary

The current code silently ignores characters after the unit as long
the unit themselves were recognized. This commit makes expand_number(3)
to fail with EINVAL if buf did not terminate after the unit character.

While I am there, also write a few test cases to validate the behavior.

MFC-after: 2 weeks

Test Plan

Run the test cases

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

lib/libutil/expand_number.c
78–81

Preexisting issue, but looks like we need to mention b in the man page.
I'm also not sure if b should mean shift = 0 or should be an optional ignored character (i.e., so that 1k and 1kb are treated identically)

91–92

it looks like this does not need a line break?

lib/libutil/tests/expand_number_test.c
49

further to my comment above I'd say this correctly tests existing behaviour, but it seems as if 1kb ought to be an accepted value

delphij added inline comments.
lib/libutil/expand_number.c
78–81

Actually I wasn't very sure why we supported b, but that was a good point. In the manual page change I'll say that the usage of b is discouraged (because some other utilities treats b as "block" and the result may be confusing).

delphij marked an inline comment as done.

Reflect reviewer comments.

Make license for test explicit per guidelines.

emaste added inline comments.
lib/libutil/tests/expand_number_test.c
54

should we check both b and B perhaps?

This revision is now accepted and ready to land.Jun 12 2023, 2:34 PM
This revision was automatically updated to reflect the committed changes.