-
Notifications
You must be signed in to change notification settings - Fork 265
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
illumos defines _MAX macros in limits.h #1266
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1266 +/- ##
==========================================
+ Coverage 51.07% 51.10% +0.03%
==========================================
Files 45 45
Lines 18250 18251 +1
==========================================
+ Hits 9321 9328 +7
+ Misses 8929 8923 -6 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The only _MAX
in rrd_list.c
is PATH_MAX
. So, this commit is about missing PATH_MAX
under illumos?
PATH_MAX
is also required in rrd_daemon.c
[1].
Therefore, could you please consider handling missing #include <limits.h>
in case of __illumos__
somewhere else, more globally?
e.g. in src/rrd_config_bottom.h
rrdtool-1.x/src/rrd_config_bottom.h
Line 30 in 39e0149
#ifndef MAXPATH |
[1]
git grep PATH_MAX
CHANGES:* Avoids potential unterminated string because of fixed PATH_MAX buffer
src/rrd_config_bottom.h:# ifdef PATH_MAX
src/rrd_config_bottom.h:# define MAXPATH PATH_MAX
src/rrd_daemon.c: if ((end_ptr - start_ptr + strlen(fullpath) + 1) >= PATH_MAX) {
src/rrd_list.c: if (strlen(dirname) + strlen(entry->d_name) + 1 >= PATH_MAX) {
Thank you for your review. You are right, it's much better to put it into |
thanks |
No description provided.