You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2022/07/12 21:05:11 UTC
[GitHub] [incubator-nuttx] ALTracer opened a new pull request, #6605: fs/mount: "df -h" fixes regarding datatype sizes
ALTracer opened a new pull request, #6605:
URL: https://github.com/apache/incubator-nuttx/pull/6605
## Summary
This PR intends to correct `df` and `df -h` nshlib command output.
Refer to #5253 for details.
## Impact
32-bit arches with HAVE_LONG_LONG (and FS_LARGEFILE) but with disabled LIBC_LONG_LONG.
## Testing
builds on ARMv7-M `nucleo-h743zi2:jumbo` with config tweaks -LIBC_LONG_LONG, +DEBUG_FS_WARN
builds and runs on ARMv6-M `stm32f072b-disco:nsh` with config tweaks; on Milandr MDR32F1QI;
only procfs tested, but now there's no garbage on stdout.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] ALTracer commented on pull request #6605: fs/mount: "df -h" fixes regarding datatype sizes
Posted by GitBox <gi...@apache.org>.
ALTracer commented on PR #6605:
URL: https://github.com/apache/incubator-nuttx/pull/6605#issuecomment-1186211678
I have a feeling libc printf will get refactored such that this PR becomes useless. I will copy the branch in my fork for history and squash-rebase this branch, leaving only the `printflike` change.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] ALTracer commented on a diff in pull request #6605: fs/mount: "df -h" fixes regarding datatype sizes
Posted by GitBox <gi...@apache.org>.
ALTracer commented on code in PR #6605:
URL: https://github.com/apache/incubator-nuttx/pull/6605#discussion_r921966422
##########
fs/mount/fs_procfs_mount.c:
##########
@@ -256,11 +261,39 @@ static int blocks_entry(FAR const char *mountpoint,
/* Generate blocks list one line at a time */
- mount_sprintf(info, "%6lu %10" PRIuOFF " %10" PRIuOFF
+#ifdef CONFIG_LIBC_LONG_LONG
+ mount_sprintf(info, "%6zu %10" PRIuOFF " %10" PRIuOFF
" %10" PRIuOFF " %s\n",
statbuf->f_bsize, statbuf->f_blocks,
statbuf->f_blocks - statbuf->f_bavail, statbuf->f_bavail,
mountpoint);
+#else
+# if defined(CONFIG_FS_LARGEFILE) && defined(CONFIG_HAVE_LONG_LONG)
+ if (statbuf->f_blocks > ULONG_MAX)
+ {
+ fwarn("Detected unsigned overflow in 'Blocks', upper word = %lu\n",
+ (uint32_t)(statbuf->f_blocks >> 32));
+ }
+
+ if ((statbuf->f_blocks - statbuf->f_bavail) > ULONG_MAX)
+ {
+ fwarn("Detected unsigned overflow in 'Used', upper word = %lu\n",
+ (uint32_t)((statbuf->f_blocks - statbuf->f_bavail) >> 32));
+ }
+
+ if (statbuf->f_bavail > ULONG_MAX)
+ {
+ fwarn("Detected unsigned overflow in 'Avail', upper word = %lu\n",
Review Comment:
Oh, yes, that is clever. I had something similar in mind while staring at three nearly identical strings. I'm just not as proficient in juggling format-strings like that. Will test locally.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #6605: fs/mount: "df -h" fixes regarding datatype sizes
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6605:
URL: https://github.com/apache/incubator-nuttx/pull/6605#discussion_r922686163
##########
fs/mount/fs_procfs_mount.c:
##########
@@ -256,11 +261,39 @@ static int blocks_entry(FAR const char *mountpoint,
/* Generate blocks list one line at a time */
- mount_sprintf(info, "%6lu %10" PRIuOFF " %10" PRIuOFF
+#ifdef CONFIG_LIBC_LONG_LONG
+ mount_sprintf(info, "%6zu %10" PRIuOFF " %10" PRIuOFF
" %10" PRIuOFF " %s\n",
statbuf->f_bsize, statbuf->f_blocks,
statbuf->f_blocks - statbuf->f_bavail, statbuf->f_bavail,
mountpoint);
+#else
+# if defined(CONFIG_FS_LARGEFILE) && defined(CONFIG_HAVE_LONG_LONG)
+ if (statbuf->f_blocks > ULONG_MAX)
+ {
+ fwarn("Detected unsigned overflow in 'Blocks', upper word = %lu\n",
+ (uint32_t)(statbuf->f_blocks >> 32));
+ }
+
+ if ((statbuf->f_blocks - statbuf->f_bavail) > ULONG_MAX)
+ {
+ fwarn("Detected unsigned overflow in 'Used', upper word = %lu\n",
+ (uint32_t)((statbuf->f_blocks - statbuf->f_bavail) >> 32));
+ }
+
+ if (statbuf->f_bavail > ULONG_MAX)
+ {
+ fwarn("Detected unsigned overflow in 'Avail', upper word = %lu\n",
Review Comment:
Let's move the warning to the printf level instead.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] acassis commented on a diff in pull request #6605: fs/mount: "df -h" fixes regarding datatype sizes
Posted by GitBox <gi...@apache.org>.
acassis commented on code in PR #6605:
URL: https://github.com/apache/incubator-nuttx/pull/6605#discussion_r921539773
##########
fs/mount/fs_procfs_mount.c:
##########
@@ -268,6 +268,26 @@ static int blocks_entry(FAR const char *mountpoint,
statbuf->f_blocks - statbuf->f_bavail, statbuf->f_bavail,
mountpoint);
#else
+# if defined(CONFIG_FS_LARGEFILE) && defined(CONFIG_HAVE_LONG_LONG)
Review Comment:
@ALTracer I'm confuse here, shouldn't be if defined(CONFIG_FS_LARGEFILE) && defined(CONFIG_LIBC_LONG_LONG)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] ALTracer commented on a diff in pull request #6605: fs/mount: "df -h" fixes regarding datatype sizes
Posted by GitBox <gi...@apache.org>.
ALTracer commented on code in PR #6605:
URL: https://github.com/apache/incubator-nuttx/pull/6605#discussion_r920972983
##########
fs/mount/fs_procfs_mount.c:
##########
@@ -101,6 +101,7 @@ struct mount_info_s
/* Helpers */
+printflike(2, 3)
Review Comment:
Agreed. Will move to the end like in the other header files. In next commit.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] ALTracer commented on pull request #6605: fs/mount: "df -h" fixes regarding datatype sizes
Posted by GitBox <gi...@apache.org>.
ALTracer commented on PR #6605:
URL: https://github.com/apache/incubator-nuttx/pull/6605#issuecomment-1186596848
> The change is included by a bigger patch here: #6632. Please take a look.
Oh come on. Fine.
There's nothing left of this PR as it tried to deal with the issue at hand as a special one-sided case. I'm content with closing it unmerged.
At least it has lead to a couple system-wide changes which might enhance RTOS behaviour on next release.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] ALTracer closed pull request #6605: fs/mount: "df -h" fixes regarding datatype sizes
Posted by GitBox <gi...@apache.org>.
ALTracer closed pull request #6605: fs/mount: "df -h" fixes regarding datatype sizes
URL: https://github.com/apache/incubator-nuttx/pull/6605
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] ALTracer commented on a diff in pull request #6605: fs/mount: "df -h" fixes regarding datatype sizes
Posted by GitBox <gi...@apache.org>.
ALTracer commented on code in PR #6605:
URL: https://github.com/apache/incubator-nuttx/pull/6605#discussion_r921914940
##########
fs/mount/fs_procfs_mount.c:
##########
@@ -248,6 +248,10 @@ static int blocks_entry(FAR const char *mountpoint,
if (!info->header)
{
+#if defined(CONFIG_HAVE_LONG_LONG) && !defined(CONFIG_LIBC_LONG_LONG)
+ finfo("long long is enabled, but libc does not support it;\n"
Review Comment:
@mlyszczek proposed it in #5253. I can change this (no-op on production builds) to a preprocessor warning. We cannot know predict at compile time what size sdcards will be inserted at run-time. Oh well, that should be handled by the next fwarn()'s.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] ALTracer commented on a diff in pull request #6605: fs/mount: "df -h" fixes regarding datatype sizes
Posted by GitBox <gi...@apache.org>.
ALTracer commented on code in PR #6605:
URL: https://github.com/apache/incubator-nuttx/pull/6605#discussion_r921872494
##########
fs/mount/fs_procfs_mount.c:
##########
@@ -268,6 +268,26 @@ static int blocks_entry(FAR const char *mountpoint,
statbuf->f_blocks - statbuf->f_bavail, statbuf->f_bavail,
mountpoint);
#else
+# if defined(CONFIG_FS_LARGEFILE) && defined(CONFIG_HAVE_LONG_LONG)
Review Comment:
Well no, in this branch I'm trying to gracefully cast down and print as ul, but still check first for possible overflow since I can compute in ull.
This would be impossible if HAVE_LONG_LONG got disabled.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #6605: fs/mount: "df -h" fixes regarding datatype sizes
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6605:
URL: https://github.com/apache/incubator-nuttx/pull/6605#discussion_r921379827
##########
fs/mount/fs_procfs_mount.c:
##########
@@ -257,11 +261,19 @@ static int blocks_entry(FAR const char *mountpoint,
/* Generate blocks list one line at a time */
+#ifdef CONFIG_LIBC_LONG_LONG
Review Comment:
don't need change with https://github.com/apache/incubator-nuttx/pull/6613
##########
fs/mount/fs_procfs_mount.c:
##########
@@ -101,6 +101,7 @@ struct mount_info_s
/* Helpers */
+printflike(2, 3)
Review Comment:
But it isn't good to leave the temporary change in git history.
##########
fs/mount/fs_procfs_mount.c:
##########
@@ -248,6 +248,10 @@ static int blocks_entry(FAR const char *mountpoint,
if (!info->header)
{
+#if defined(CONFIG_HAVE_LONG_LONG) && !defined(CONFIG_LIBC_LONG_LONG)
+ finfo("long long is enabled, but libc does not support it;\n"
Review Comment:
it's strange to print log here, or you can use static_assert instead.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #6605: fs/mount: "df -h" fixes regarding datatype sizes
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on PR #6605:
URL: https://github.com/apache/incubator-nuttx/pull/6605#issuecomment-1186413479
> I have a feeling libc printf will get refactored such that this PR becomes useless. I will copy the branch in my fork for history and squash-rebase this branch, leaving only the `printflike` change.
The change is included by a bigger patch here: https://github.com/apache/incubator-nuttx/pull/6632. Please take a look.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #6605: fs/mount: "df -h" fixes regarding datatype sizes
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6605:
URL: https://github.com/apache/incubator-nuttx/pull/6605#discussion_r921875476
##########
fs/mount/fs_procfs_mount.c:
##########
@@ -248,6 +248,10 @@ static int blocks_entry(FAR const char *mountpoint,
if (!info->header)
{
+#if defined(CONFIG_HAVE_LONG_LONG) && !defined(CONFIG_LIBC_LONG_LONG)
+ finfo("long long is enabled, but libc does not support it;\n"
Review Comment:
Yes, but can be a compile time `#warning` instead of console printf
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] mlyszczek commented on a diff in pull request #6605: fs/mount: "df -h" fixes regarding datatype sizes
Posted by GitBox <gi...@apache.org>.
mlyszczek commented on code in PR #6605:
URL: https://github.com/apache/incubator-nuttx/pull/6605#discussion_r921937388
##########
fs/mount/fs_procfs_mount.c:
##########
@@ -256,11 +261,39 @@ static int blocks_entry(FAR const char *mountpoint,
/* Generate blocks list one line at a time */
- mount_sprintf(info, "%6lu %10" PRIuOFF " %10" PRIuOFF
+#ifdef CONFIG_LIBC_LONG_LONG
+ mount_sprintf(info, "%6zu %10" PRIuOFF " %10" PRIuOFF
" %10" PRIuOFF " %s\n",
statbuf->f_bsize, statbuf->f_blocks,
statbuf->f_blocks - statbuf->f_bavail, statbuf->f_bavail,
mountpoint);
+#else
+# if defined(CONFIG_FS_LARGEFILE) && defined(CONFIG_HAVE_LONG_LONG)
+ if (statbuf->f_blocks > ULONG_MAX)
+ {
+ fwarn("Detected unsigned overflow in 'Blocks', upper word = %lu\n",
+ (uint32_t)(statbuf->f_blocks >> 32));
+ }
+
+ if ((statbuf->f_blocks - statbuf->f_bavail) > ULONG_MAX)
+ {
+ fwarn("Detected unsigned overflow in 'Used', upper word = %lu\n",
+ (uint32_t)((statbuf->f_blocks - statbuf->f_bavail) >> 32));
+ }
+
+ if (statbuf->f_bavail > ULONG_MAX)
+ {
+ fwarn("Detected unsigned overflow in 'Avail', upper word = %lu\n",
Review Comment:
As explained in my previous comment - I think I would use fprintf(stderr) in all of these cases. If something should fail - it should fail as soon as possible and as loud as possible. fwarn() is not particularly loud - you need to enable like 5 Kconfig options to get that. And if someone sees strange output in df, I don't think their first though will be to enable fwarn() and they will simply waste time.
The only concern is wasting flash memory, but that can be optimized by doing
```
#define OVERFLOW_FMT "Detected unsigned overflow in '%s', upper word = %lu\n"
fprintf(stderr, OVERFLOW_FMT, "Blocks", ...);
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #6605: fs/mount: "df -h" fixes regarding datatype sizes
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6605:
URL: https://github.com/apache/incubator-nuttx/pull/6605#discussion_r919612925
##########
fs/mount/fs_procfs_mount.c:
##########
@@ -248,6 +248,10 @@ static int blocks_entry(FAR const char *mountpoint,
if (!info->header)
{
+#if defined(CONFIG_HAVE_LONG_LONG) && !defined(CONFIG_LIBC_LONG_LONG)
Review Comment:
since the code increase of long long is very little, look like it's better to remove CONFIG_LIBC_LONG_LONG and use CONFIG_HAVE_LONG_LONG instead.
##########
fs/mount/fs_procfs_mount.c:
##########
@@ -101,6 +101,7 @@ struct mount_info_s
/* Helpers */
+printflike(2, 3)
Review Comment:
move to the end
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #6605: fs/mount: "df -h" fixes regarding datatype sizes
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6605:
URL: https://github.com/apache/incubator-nuttx/pull/6605#discussion_r922685607
##########
fs/mount/fs_procfs_mount.c:
##########
@@ -248,6 +248,10 @@ static int blocks_entry(FAR const char *mountpoint,
if (!info->header)
{
+#if defined(CONFIG_HAVE_LONG_LONG) && !defined(CONFIG_LIBC_LONG_LONG)
+ finfo("long long is enabled, but libc does not support it;\n"
Review Comment:
There are many places which call [s]printf with [u]int64_t, so it isn't good to check it only here or spread the check around the whole code base. The good fix is do the check inside the core of printf like https://github.com/apache/incubator-nuttx/pull/6613.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] ALTracer commented on a diff in pull request #6605: fs/mount: "df -h" fixes regarding datatype sizes
Posted by GitBox <gi...@apache.org>.
ALTracer commented on code in PR #6605:
URL: https://github.com/apache/incubator-nuttx/pull/6605#discussion_r920977116
##########
fs/mount/fs_procfs_mount.c:
##########
@@ -248,6 +248,10 @@ static int blocks_entry(FAR const char *mountpoint,
if (!info->header)
{
+#if defined(CONFIG_HAVE_LONG_LONG) && !defined(CONFIG_LIBC_LONG_LONG)
Review Comment:
Not really. I would like 32-bit devices to still calculate with ull but print via ul, because some devices are (implied to be) unlikely to have to deal with large filesystems and block counts. They don't *require* LIBC_LONG_LONG in principle, no matter the code size increase (and how we measure it).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] acassis commented on a diff in pull request #6605: fs/mount: "df -h" fixes regarding datatype sizes
Posted by GitBox <gi...@apache.org>.
acassis commented on code in PR #6605:
URL: https://github.com/apache/incubator-nuttx/pull/6605#discussion_r921539773
##########
fs/mount/fs_procfs_mount.c:
##########
@@ -268,6 +268,26 @@ static int blocks_entry(FAR const char *mountpoint,
statbuf->f_blocks - statbuf->f_bavail, statbuf->f_bavail,
mountpoint);
#else
+# if defined(CONFIG_FS_LARGEFILE) && defined(CONFIG_HAVE_LONG_LONG)
Review Comment:
@ALTracer I'm confuse here, shouldn't be ```if defined(CONFIG_FS_LARGEFILE) && defined(CONFIG_LIBC_LONG_LONG)```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] mlyszczek commented on a diff in pull request #6605: fs/mount: "df -h" fixes regarding datatype sizes
Posted by GitBox <gi...@apache.org>.
mlyszczek commented on code in PR #6605:
URL: https://github.com/apache/incubator-nuttx/pull/6605#discussion_r921930843
##########
fs/mount/fs_procfs_mount.c:
##########
@@ -248,6 +248,10 @@ static int blocks_entry(FAR const char *mountpoint,
if (!info->header)
{
+#if defined(CONFIG_HAVE_LONG_LONG) && !defined(CONFIG_LIBC_LONG_LONG)
+ finfo("long long is enabled, but libc does not support it;\n"
Review Comment:
#warning is not a good solution IMO. If I know what I'm doing (like 100% sure not using >4gb card) I wouldn't want to have warnings every time I compile code.
If we have ll and no ll support in libc, we can detect overflow in runtime and then unconditionally print warning on stderr. Performance penalty is negligible - I mean, how often would you call df in production code?
I suspect finfo won't be seen by anyone. Someone will see strange output, will start to investigate, a lot of people probably won't think about turning on finfo and will simply waste time.
I think unconditional print to stderr about overflow is best - if we are going to fail, we should do it as soon as possible and as loud as possible, not close it behind multiple Kconfig options
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] ALTracer commented on a diff in pull request #6605: fs/mount: "df -h" fixes regarding datatype sizes
Posted by GitBox <gi...@apache.org>.
ALTracer commented on code in PR #6605:
URL: https://github.com/apache/incubator-nuttx/pull/6605#discussion_r922688176
##########
fs/mount/fs_procfs_mount.c:
##########
@@ -248,6 +248,10 @@ static int blocks_entry(FAR const char *mountpoint,
if (!info->header)
{
+#if defined(CONFIG_HAVE_LONG_LONG) && !defined(CONFIG_LIBC_LONG_LONG)
+ finfo("long long is enabled, but libc does not support it;\n"
Review Comment:
Yes, that's better, if you are sure about refactoring core printf.
I will drop this proposed check from fs/mount then. And always pass uint64_t.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #6605: fs/mount: "df -h" fixes regarding datatype sizes
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on PR #6605:
URL: https://github.com/apache/incubator-nuttx/pull/6605#issuecomment-1186742942
Let's improve together.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] ALTracer commented on a diff in pull request #6605: fs/mount: "df -h" fixes regarding datatype sizes
Posted by GitBox <gi...@apache.org>.
ALTracer commented on code in PR #6605:
URL: https://github.com/apache/incubator-nuttx/pull/6605#discussion_r921874715
##########
fs/mount/fs_procfs_mount.c:
##########
@@ -248,6 +248,10 @@ static int blocks_entry(FAR const char *mountpoint,
if (!info->header)
{
+#if defined(CONFIG_HAVE_LONG_LONG) && !defined(CONFIG_LIBC_LONG_LONG)
+ finfo("long long is enabled, but libc does not support it;\n"
Review Comment:
finfo is debugging output, activated with CONFIG_DEBUG_FS_INFO, and is directed at developers early in prototyping, who are trying to work with **large** filesystems without enabling LONG_LONG.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #6605: fs/mount: "df -h" fixes regarding datatype sizes
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6605:
URL: https://github.com/apache/incubator-nuttx/pull/6605#discussion_r922685607
##########
fs/mount/fs_procfs_mount.c:
##########
@@ -248,6 +248,10 @@ static int blocks_entry(FAR const char *mountpoint,
if (!info->header)
{
+#if defined(CONFIG_HAVE_LONG_LONG) && !defined(CONFIG_LIBC_LONG_LONG)
+ finfo("long long is enabled, but libc does not support it;\n"
Review Comment:
There are many places which call [s]printf with [u]int64_t, so it isn't good to check it only here or spread the check around the whole code base. The good fix is do the check inside the core of printf like https://github.com/apache/incubator-nuttx/pull/6613 which should address all issue you concern about:
1. Output the correct value if the value isn't bigger than [U]LONG_MAX
2. Assert if the value is out of range.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org