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 2021/07/21 06:19:23 UTC
[GitHub] [incubator-nuttx] xiaoxiang781216 opened a new pull request #4193: fs: Change off_t and related types to int64_t if long long is supported
xiaoxiang781216 opened a new pull request #4193:
URL: https://github.com/apache/incubator-nuttx/pull/4193
## Summary
since it is very popular that the storage capcacity is large than 4GB
even in the embedded system:
https://www.opengroup.org/platform/lfs.html
https://en.wikipedia.org/wiki/Large-file_support
## Impact
Support 64bit offset in vfs layer, each file system need be reviewed individually to avoid the hardcode 32bit offset type if it design to support the large file.
## Testing
--
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] patacongo edited a comment on pull request #4193: fs: Change off_t and related types to int64_t if long long is supported
Posted by GitBox <gi...@apache.org>.
patacongo edited a comment on pull request #4193:
URL: https://github.com/apache/incubator-nuttx/pull/4193#issuecomment-884201118
Another issue is that some of the older file system APIs do not use off_t but instead `long int` to represent file position. For example:
stdio.h:long ftell(FAR FILE *stream);
stdio.h:off_t ftello(FAR FILE *stream);
Also:
stdio.h:int fseek(FAR FILE *stream, long int offset, int whence);
stdio.h:int fseeko(FAR FILE *stream, off_t offset, int whence);
--
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] patacongo commented on pull request #4193: fs: Change off_t and related types to int64_t if long long is supported
Posted by GitBox <gi...@apache.org>.
patacongo commented on pull request #4193:
URL: https://github.com/apache/incubator-nuttx/pull/4193#issuecomment-884201118
Another issue is that some of the older file system APIs do not use off_t but instead `long int` to represent file position. For example:
stdio.h:long ftell(FAR FILE *stream);
stdio.h:off_t ftello(FAR FILE *stream);
--
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] gustavonihei commented on a change in pull request #4193: fs: Change off_t and related types to int64_t if long long is supported
Posted by GitBox <gi...@apache.org>.
gustavonihei commented on a change in pull request #4193:
URL: https://github.com/apache/incubator-nuttx/pull/4193#discussion_r682138910
##########
File path: arch/arm/src/cxd56xx/cxd56_sfc.c
##########
@@ -82,7 +82,8 @@ static int cxd56_erase(FAR struct mtd_dev_s *dev, off_t startblock,
int ret;
size_t i;
- finfo("erase: %08lx (%u blocks)\n", startblock << PAGE_SHIFT, nblocks);
+ finfo("erase: %" PRIxOFF " (%u blocks)\n",
Review comment:
Besides the change to the format specifier, the 8 leading zeroes also have been removed.
I am not raising an issue, just to confirm whether this was intentional or by mistake.
--
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] patacongo commented on pull request #4193: fs: Change off_t and related types to int64_t if long long is supported
Posted by GitBox <gi...@apache.org>.
patacongo commented on pull request #4193:
URL: https://github.com/apache/incubator-nuttx/pull/4193#issuecomment-884341521
> > > a compiler option turned large file support on or off (selecting the 32-bit or 64-bit APIs)
> >
> >
> > You can see the old uClibc LFS support built in (or not) here: https://git.uclibc.org/uClibc/tree/libc/stdio/Makefile.in#n22
>
> This design is to keep the binary compatibility, it isn't the goal of NuttX. But anyway, I add a new option FS_LARGEFILE which is similar to UCLIBC_HAS_LFS.
The point is that, at least in the past, LFS worked because on 32-bit platforms libc implemented standard 32-bit versions of file access functions as well as 64-bit versions. If LFS was selected, the compiler mangled the function names so that the application linked to the 64-bit versions. That answers DavidS's question about the naming of the 64-bit file access functions: the application never saw the name of the 64-bit version.
None of this is now needed with the moden 64-bit desktop CPUs.
--
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] patacongo commented on pull request #4193: fs: Change off_t and related types to int64_t if long long is supported
Posted by GitBox <gi...@apache.org>.
patacongo commented on pull request #4193:
URL: https://github.com/apache/incubator-nuttx/pull/4193#issuecomment-884196038
If off_t changes from anything but 32-bits, we also need to fix all file systems that expect off_t to be 32-bits in the binary meta-data of the file. These all need to be checked (and maybe others):
fs/fat/
fs/nxffs/
fs/smartfs/
fs/spiffs/
fs/tmpfs/
And possibly data structures in:
include/nuttx/fs/dirent.h
include/nuttx/fs/loop.h
include/nuttx/fs/smart.h
include/nuttx/fs/userfs.h
And this would need to be kept in sync:
include/nuttx/fs/hostfs.h:typedef int32_t nuttx_off_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 #4193: fs: Change off_t and related types to int64_t if long long is supported
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #4193:
URL: https://github.com/apache/incubator-nuttx/pull/4193#issuecomment-891814417
> @xiaoxiang781216 - What about all the apps changes that were done and committed already? Would you do a app's PR that propagates the format changes to all those as well?
No. First, there is no data lose since the code cast off_t to intmax_t. Second, only two apps(examples/media and fsutils/mkfatfs) get affected.
--
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] patacongo edited a comment on pull request #4193: fs: Change off_t and related types to int64_t if long long is supported
Posted by GitBox <gi...@apache.org>.
patacongo edited a comment on pull request #4193:
URL: https://github.com/apache/incubator-nuttx/pull/4193#issuecomment-884333212
Another thing that we will have to be careful with is in reading and writing 64-bit values. On a 32-bit ARMv7-M CPU, 64-bit access are not atomic. The CPU can be interrupted or the task can be suspended between each 32-bit access and this may result in a corrupted access.
Every 64-bit access must be protected so that it completes without modification at the interrupt level or by other tasks. sched_lock() is probably sufficient in most non-SMP cases. Inside of file system logic, re-entrancy protections may also be sufficient (?)
64-bit accesses are atomic on ARMv7-A and probably other ARMs and other CPU architectures. Unfornately, ARMv7-M is most prevalent in the 32-bit embedded world.
--
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 #4193: fs: Change off_t and related types to int64_t if long long is supported
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #4193:
URL: https://github.com/apache/incubator-nuttx/pull/4193#issuecomment-884291831
> Sent from my GalaxyPOSIX provides off64_t and a whole set of large file tile APIs for this. Off_t is traditionally 32 bits
opengroup doesn't define off64_t and lseek64 related API. What's opengroup expect is:
1. typedef off_t to 32bit, the file size limit to 4GB
2. typedef off_t to 64bit, the file size has no limitation
since opengroup only care about the source code compatibility, but not the binary compatibility. On the other hand, it's a hard requirement to keep the binary compatibility for all popular OS. So, these OS have to keep off_t as 32bit and add off64_t/lseek64.
> > @patacongo I see https://www.opengroup.org/platform/lfs.html, but where is the API list you mention?
>
> I am not an LFS expert, but the Wikipedia article kind of explains things: https://en.wikipedia.org/wiki/Large-file_support . Basically, (1) a compiler option turned large file support on or off (selecting the 32-bit or 64-bit APIs) and (2) no one cares any more since 64-bit machines rule the desktop.
>
> Linux has O_LARGEFILE that can select when a file is opened. https://man7.org/linux/man-pages/man2/open.2.html
Since NuttX never claim the binary compatibility as the goal, typedef off_t to int64_t is an clean and compliant implementation. The xxx64 macro just make libc more compatible with popular desktop OS:
https://linux.die.net/man/3/lseek64
http://www.qnx.com/developers/docs/7.0.0/#com.qnx.doc.neutrino.lib_ref/topic/l/lseek.html
> If off_t changes from anything but 32-bits, we also need to fix all file systems that expect off_t to be 32-bits in the binary meta-data of the file. These all need to be checked (and maybe others):
>
> ```
> fs/fat/
> fs/nxffs/
> fs/smartfs/
> fs/spiffs/
> fs/tmpfs/
> ```
>
As I mention in the summary:
1. If the file system support 64bit file natively, we need review it's code to change all 32bit type to 64bit
2. If the file system doesn't support 64bit file, we don't need change anything here since it's impossible to generate a file > 4GB.
> And possibly data structures in:
>
> ```
> include/nuttx/fs/dirent.h
> include/nuttx/fs/loop.h
> include/nuttx/fs/smart.h
> include/nuttx/fs/userfs.h
> ```
>
> And this would need to be kept in sync:
>
> ```
> include/nuttx/fs/hostfs.h:typedef int32_t nuttx_off_t;
> ```
I will review the general file sysem(e.g. hostfs and userfs) and update if needed.
> > a compiler option turned large file support on or off (selecting the 32-bit or 64-bit APIs)
>
> You can see the old uClibc LFS support built in (or not) here: https://git.uclibc.org/uClibc/tree/libc/stdio/Makefile.in#n22
This design is to keep the binary compatibility, it isn't the goal of NuttX. But anyway, I add a new option FS_LARGEFILE which is similar to UCLIBC_HAS_LFS.
> Another issue is that some of the older file system APIs do not use off_t but instead `long int` to represent file position. For example:
>
> ```
> stdio.h:long ftell(FAR FILE *stream);
> stdio.h:off_t ftello(FAR FILE *stream);
> ```
>
> Also:
>
> ```
> stdio.h:int fseek(FAR FILE *stream, long int offset, int whence);
> stdio.h:int fseeko(FAR FILE *stream, off_t offset, int whence);
> ```
Yes, this is the design flaw and why the standard add ftello/fseeko, please see the below link:
https://unix.org/version2/whatsnew/lfs20mar.html
--
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] patacongo commented on a change in pull request #4193: fs: Change off_t and related types to int64_t if long long is supported
Posted by GitBox <gi...@apache.org>.
patacongo commented on a change in pull request #4193:
URL: https://github.com/apache/incubator-nuttx/pull/4193#discussion_r675649527
##########
File path: include/sys/types.h
##########
@@ -175,6 +183,16 @@ typedef int wint_t;
typedef int wctype_t;
+#if defined(CONFIG_FS_LARGEFILE) && defined(CONFIG_HAVE_LONG_LONG)
+/* Large file versions */
+
+typedef uint64_t fsblkcnt_t;
+typedef uint64_t fsfilcnt_t;
+
+typedef uint64_t blkcnt_t;
+typedef int64_t off_t;
Review comment:
block counts and file counts don't have anything to do with large file support, do they? The are related to volume size, but that is a completely different thing from file size.
--
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 edited a comment on pull request #4193: fs: Change off_t and related types to int64_t if long long is supported
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 edited a comment on pull request #4193:
URL: https://github.com/apache/incubator-nuttx/pull/4193#issuecomment-884373965
Here is the size difference on stm32f4discovery:nsh:
```
arm-none-eabi-size nuttx.lfs
text data bss dec hex filename
70332 108 1588 72028 1195c nuttx.lfs
arm-none-eabi-size nuttx
text data bss dec hex filename
69888 108 1588 71584 117a0 nuttx
```
The LFS version increase 444B(0.6%). @davids5 @patacongo do you think FS_LARGEFILE is still really needed?
--
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 pull request #4193: fs: Change off_t and related types to int64_t if long long is supported
Posted by GitBox <gi...@apache.org>.
acassis commented on pull request #4193:
URL: https://github.com/apache/incubator-nuttx/pull/4193#issuecomment-884446225
@xiaoxiang781216 you could define is as default y if !CONFIG_DEFAULT_SMALL
--
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] patacongo commented on pull request #4193: fs: Change off_t and related types to int64_t if long long is supported
Posted by GitBox <gi...@apache.org>.
patacongo commented on pull request #4193:
URL: https://github.com/apache/incubator-nuttx/pull/4193#issuecomment-884199760
> a compiler option turned large file support on or off (selecting the 32-bit or 64-bit APIs)
You can see the old uClibc LFS support built in (or not) here: https://git.uclibc.org/uClibc/tree/libc/stdio/Makefile.in#n22
--
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] davids5 edited a comment on pull request #4193: fs: Change off_t and related types to int64_t if long long is supported
Posted by GitBox <gi...@apache.org>.
davids5 edited a comment on pull request #4193:
URL: https://github.com/apache/incubator-nuttx/pull/4193#issuecomment-885718062
@xiaoxiang781216 Thank you for all the work and problem solving. I can test this on Monday.
Please have one APP PR with all the commits that need to support this PR. Please this PR + the APPs PR pass your local CI.
Also rebase on master and list the deltas in size from master to this PR w/wo the feature added.
I can not test the Zilog build, But I will run it on PX4 CI and HW CI.
--
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 #4193: fs: Change off_t and related types to int64_t if long long is supported
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #4193:
URL: https://github.com/apache/incubator-nuttx/pull/4193#issuecomment-884376930
> Another thing that we will have to be careful with is in reading and writing 64-bit values. On a 32-bit ARMv7-M CPU, 64-bit access are not atomic. The CPU can be interrupted or the task can be suspended between each 32-bit access and this may result in a corrupted access.
>
> Every 64-bit access must be protected so that it completes without modification at the interrupt level or by other tasks. sched_lock() is probably sufficient in most non-SMP cases. Inside of file system logic, re-entrancy protections may also be sufficient (?)
>
> 64-bit accesses are atomic on ARMv7-A and probably other ARMs and other CPU architectures. Unfornately, ARMv7-M is most prevalent in the 32-bit embedded world.
Even off_t is 32bit, the programmer still need be careful in the multithread environment, since fs driver normally add the write/read len to the current offset, but += operator is never atomic on any hardware.
--
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] davids5 commented on pull request #4193: fs: Change off_t and related types to int64_t if long long is supported
Posted by GitBox <gi...@apache.org>.
davids5 commented on pull request #4193:
URL: https://github.com/apache/incubator-nuttx/pull/4193#issuecomment-884406568
> Here is the size difference on stm32f4discovery:nsh:
>
> ```
> arm-none-eabi-size nuttx.lfs
> text data bss dec hex filename
> 70332 108 1588 72028 1195c nuttx.lfs
> arm-none-eabi-size nuttx
> text data bss dec hex filename
> 69888 108 1588 71584 117a0 nuttx
> ```
>
> The LFS version increase 444B(0.6%). @davids5 @patacongo do you think FS_LARGEFILE is still really needed?
If it the last 444 byes of free flash for a unused feature yes it needs to be switchable.
What are the down size of having the Kconfig?
--
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 #4193: fs: Change off_t and related types to int64_t if long long is supported
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #4193:
URL: https://github.com/apache/incubator-nuttx/pull/4193#issuecomment-884369694
> > > > a compiler option turned large file support on or off (selecting the 32-bit or 64-bit APIs)
> > >
> > >
> > > You can see the old uClibc LFS support built in (or not) here: https://git.uclibc.org/uClibc/tree/libc/stdio/Makefile.in#n22
> >
> >
> > This design is to keep the binary compatibility, it isn't the goal of NuttX. But anyway, I add a new option FS_LARGEFILE which is similar to UCLIBC_HAS_LFS.
>
> The point is that, at least in the past, LFS worked because on 32-bit platforms libc implemented standard 32-bit versions of file access functions as well as 64-bit versions. If LFS was selected, the compiler mangled the function names so that the application linked to the 64-bit versions. That answers DavidS's question about the naming of the 64-bit file access functions: the application never saw the name of the 64-bit version.
>
> None of this is now needed with the moden 64-bit desktop CPUs.
Yes, but all fs syscall and driver inside linux kernel already switch to 64bit version even on 32bit arch. The only reason that libc keep 32bit version just want to keep the binary compatiblity. If you look at the new libc implementation after LFS become the standard, there is no 32bit version at all regoardless whether the target platform is 32bit or 64bit:
https://github.com/bminor/musl/blob/master/include/alltypes.h.in#L29
https://github.com/bminor/musl/blob/master/include/unistd.h#L42
https://github.com/bminor/musl/blob/master/include/unistd.h#L197-L205
musl is a popular libc library and is used on many 32bit embedded platform. Actually, my change reference how musl do the same thing.
--
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] davids5 commented on pull request #4193: fs: Change off_t and related types to int64_t if long long is supported
Posted by GitBox <gi...@apache.org>.
davids5 commented on pull request #4193:
URL: https://github.com/apache/incubator-nuttx/pull/4193#issuecomment-891836677
> > @xiaoxiang781216 - What about all the apps changes that were done and committed already? Would you do a app's PR that propagates the format changes to all those as well?
>
> No. First, there is no data lose since the code cast off_t to intmax_t. Second, only two apps(examples/media and fsutils/mkfatfs) get affected.
What about constancy and making sure the correct patterns are replicated in the future?
--
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 #4193: fs: Change off_t and related types to int64_t if long long is supported
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #4193:
URL: https://github.com/apache/incubator-nuttx/pull/4193#issuecomment-884641306
> If it the last 444 byes of free flash for a unused feature yes it needs to be switchable.
>
> What are the down size of having the Kconfig?
More config will make the system hard to use especially for new user. We need balance here. @acassis CONFIG_DEFAULT_SMALL is a good idea.
--
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] gustavonihei commented on a change in pull request #4193: fs: Change off_t and related types to int64_t if long long is supported
Posted by GitBox <gi...@apache.org>.
gustavonihei commented on a change in pull request #4193:
URL: https://github.com/apache/incubator-nuttx/pull/4193#discussion_r682139268
##########
File path: arch/arm/src/cxd56xx/cxd56_sfc.c
##########
@@ -101,7 +102,8 @@ static ssize_t cxd56_bread(FAR struct mtd_dev_s *dev, off_t startblock,
{
int ret;
- finfo("bread: %08lx (%u blocks)\n", startblock << PAGE_SHIFT, nblocks);
+ finfo("bread: %" PRIxOFF "(%u blocks)\n",
Review comment:
```suggestion
finfo("bread: %" PRIxOFF " (%u blocks)\n",
```
I believe there is a missing whitespace here.
--
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] patacongo commented on a change in pull request #4193: fs: Change off_t and related types to int64_t if long long is supported
Posted by GitBox <gi...@apache.org>.
patacongo commented on a change in pull request #4193:
URL: https://github.com/apache/incubator-nuttx/pull/4193#discussion_r675581516
##########
File path: include/nuttx/fs/hostfs.h
##########
@@ -106,8 +106,13 @@ typedef int16_t nuttx_uid_t;
typedef uint16_t nuttx_dev_t;
typedef uint16_t nuttx_ino_t;
typedef uint16_t nuttx_nlink_t;
+#ifdef CONFIG_FS_LARGEFILE
+typedef int64_t nuttx_off_t;
Review comment:
A cleaner solution would be to move the selection of CONFIG_HAVE_LONG_LONG to a Kconfig file. In fact, all of compiler.h could be move to Kconfig files. This solution is fine with me, however.
--
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 #4193: fs: Change off_t and related types to int64_t if long long is supported
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #4193:
URL: https://github.com/apache/incubator-nuttx/pull/4193#issuecomment-891557618
@davids5 Done.
--
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] patacongo commented on pull request #4193: fs: Change off_t and related types to int64_t if long long is supported
Posted by GitBox <gi...@apache.org>.
patacongo commented on pull request #4193:
URL: https://github.com/apache/incubator-nuttx/pull/4193#issuecomment-884143449
Sent from my GalaxyPOSIX provides off64_t and a whole set of large file tile APIs for this. Off_t is traditionally 32 bits
--
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 #4193: fs: Change off_t and related types to int64_t if long long is supported
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #4193:
URL: https://github.com/apache/incubator-nuttx/pull/4193#issuecomment-891900411
PRIxOFF is a nuttx specific definition, is it good to modify the code to use 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] xiaoxiang781216 edited a comment on pull request #4193: fs: Change off_t and related types to int64_t if long long is supported
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 edited a comment on pull request #4193:
URL: https://github.com/apache/incubator-nuttx/pull/4193#issuecomment-885746634
> @xiaoxiang781216 Thank you for all the work and problem solving. I can test this on Monday.
>
> Please have one APP PR with all the commits that need to support this PR. Please have this PR + the APPs PR pass your local CI.
>
There is only one patch in app: apache/incubator-nuttx-apps#812, please review merge it and then this PR can pass CI too.
> Also rebase on master and list the deltas in size from master to this PR w/wo the feature added.
>
I already list before, let copy it here again:
```
arm-none-eabi-size nuttx.lfs
text data bss dec hex filename
70332 108 1588 72028 1195c nuttx.lfs
arm-none-eabi-size nuttx
text data bss dec hex filename
69888 108 1588 71584 117a0 nuttx
```
> I can not test the Zilog build, But I will run it on PX4 CI and HW CI.
--
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 change in pull request #4193: fs: Change off_t and related types to int64_t if long long is supported
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #4193:
URL: https://github.com/apache/incubator-nuttx/pull/4193#discussion_r682245309
##########
File path: arch/arm/src/cxd56xx/cxd56_sfc.c
##########
@@ -140,7 +143,7 @@ static ssize_t cxd56_read(FAR struct mtd_dev_s *dev, off_t offset,
{
int ret;
- finfo("read: %08lx (%u bytes)\n", offset, nbytes);
+ finfo("read: %" PRIxOFF "(%u bytes)\n", offset, nbytes);
Review comment:
Done.
--
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 #4193: fs: Change off_t and related types to int64_t if long long is supported
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #4193:
URL: https://github.com/apache/incubator-nuttx/pull/4193#issuecomment-884643110
> I think that there is another issue with system calls on 32-bit platforms. Passing a 64-bit value will require two registers instead of one, so it will be necessary to use a different system call to pass the values, won't it? All of the registers passed in the system call are represented by uintptr_t which is 32-bits on ARMv7-M. An int32_t cannot be represented as a single uintptr_t and will have to be separated into two 32-bit values in the proxy and restored to a single 64-bit value in the stub.
>
> Also, do the 64-bit values need to lie on aligned, even registers?
We need adjust tools/mksyscall.c:
1. Proxy split 64bit to two 32bit before invoke sys_call
2. Sub merge two 32bit to 64bit before invoke the real function
The more hard issue is 64bit return value, the above trick can't work for the return value.
Actually, the returned value issue happen now:
```
"clock","time.h","","clock_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 #4193: fs: Change off_t and related types to int64_t if long long is supported
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #4193:
URL: https://github.com/apache/incubator-nuttx/pull/4193#issuecomment-884293737
> @xiaoxiang781216 @patacongo - How about doing this work on a branch and put keep the option behind a Kconfig?
Done, please search FS_LARGEFILE.
--
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 #4193: fs: Change off_t and related types to int64_t if long long is supported
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #4193:
URL: https://github.com/apache/incubator-nuttx/pull/4193#issuecomment-884689166
I open an issue(#4206) to track the syscall problem. The worst case(protect and kernel mode) just like CONFIG_FS_LARGEFILE option doesn't work, but file lengh <= 4GB still work as before.
@davids5 and @acassis please review the new version, which should address your concern.
--
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] patacongo edited a comment on pull request #4193: fs: Change off_t and related types to int64_t if long long is supported
Posted by GitBox <gi...@apache.org>.
patacongo edited a comment on pull request #4193:
URL: https://github.com/apache/incubator-nuttx/pull/4193#issuecomment-884333212
Another thing that we will have to be careful with is in reading and writing 64-bit values. On a 32-bit ARMv7-M CPU, 64-bit access are not atomic. The CPU can be interrupted or the task can be suspended between each 32-bit access and this may result in a corrupted access.
Every 64-bit access must be protected so that it completes without modification at the interrupt level or by other tasks. sched_lock() is probably sufficient in most non-SMP cases. Inside of file system logic, re-entrancy protections may also be sufficient (?)
--
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 change in pull request #4193: fs: Change off_t and related types to int64_t if long long is supported
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #4193:
URL: https://github.com/apache/incubator-nuttx/pull/4193#discussion_r673911505
##########
File path: include/dirent.h
##########
@@ -86,6 +86,14 @@
#define DT_LNK DTYPE_LINK
#define DT_SOCK DTYPE_SOCK
+#define dirent64 dirent
+#define readdir64 readdir
+#define readdir64_r readdir_r
+#define scandir64 scandir
+#define alphasort64 alphasort
+#define versionsort64 versionsort
+#define getdents64 getdents
+
Review comment:
getdents is Linux specific:
https://man7.org/linux/man-pages/man2/getdents.2.html
but @Donny9 is preparing a new patchset to convert dir API to the most conformable implementation in this week like musl:
https://git.musl-libc.org/cgit/musl/tree/src/dirent
The change will:
1. Convert fs/dirent to a char driver just like eventfd.
2. All dirxxx move to libs/libc/dirent
getdents will add in that patchset. The macro here is to avoid him forget to add the definition.
--
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 #4193: fs: Change off_t and related types to int64_t if long long is supported
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #4193:
URL: https://github.com/apache/incubator-nuttx/pull/4193#issuecomment-890654717
Rebase to fix the conflict, no real change. @davids5
--
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 #4193: fs: Change off_t and related types to int64_t if long long is supported
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #4193:
URL: https://github.com/apache/incubator-nuttx/pull/4193#issuecomment-885746634
> @xiaoxiang781216 Thank you for all the work and problem solving. I can test this on Monday.
>
> Please have one APP PR with all the commits that need to support this PR. Please have this PR + the APPs PR pass your local CI.
>
There is only one patch in app: apache/incubator-nuttx-apps#812, please review merge it and then this PR can pass Ci too since the change is simple and already pass CI.
> Also rebase on master and list the deltas in size from master to this PR w/wo the feature added.
>
I already list before, let copy it here again:
```
arm-none-eabi-size nuttx.lfs
text data bss dec hex filename
70332 108 1588 72028 1195c nuttx.lfs
arm-none-eabi-size nuttx
text data bss dec hex filename
69888 108 1588 71584 117a0 nuttx
```
> I can not test the Zilog build, But I will run it on PX4 CI and HW CI.
--
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 edited a comment on pull request #4193: fs: Change off_t and related types to int64_t if long long is supported
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 edited a comment on pull request #4193:
URL: https://github.com/apache/incubator-nuttx/pull/4193#issuecomment-885746634
> @xiaoxiang781216 Thank you for all the work and problem solving. I can test this on Monday.
>
> Please have one APP PR with all the commits that need to support this PR. Please have this PR + the APPs PR pass your local CI.
>
There is only one patch in app: apache/incubator-nuttx-apps#812, please review merge it and then this PR can pass CI too.
> Also rebase on master and list the deltas in size from master to this PR w/wo the feature added.
>
I already list before, let copy it here again:
```
arm-none-eabi-size nuttx.lfs
text data bss dec hex filename
70332 108 1588 72028 1195c nuttx.lfs
arm-none-eabi-size nuttx
text data bss dec hex filename
69888 108 1588 71584 117a0 nuttx
```
> I can not test the Zilog build, But I will run it on PX4 CI and HW CI.
Sure, let's wait your testing.
--
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] davids5 commented on pull request #4193: fs: Change off_t and related types to int64_t if long long is supported
Posted by GitBox <gi...@apache.org>.
davids5 commented on pull request #4193:
URL: https://github.com/apache/incubator-nuttx/pull/4193#issuecomment-884921443
>>It should be done and fully tested on real HW before it comes in to master.
Event the printf change will increase code size with FS_LARGEFILE off.
>Why? I don't see any reason that the code size if FS_LARGEFILE off. Could you explain more?
The effect can be seen in this [PR](https://github.com/apache/incubator-nuttx-apps/pull/811/files
) it cost 8 bytes (yes it is a debug path, but it still need to fit when needed)
1953165 - 1953157 = 8
This is about code space and useless bloat. If it is Off it need to cost 0
--
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 edited a comment on pull request #4193: fs: Change off_t and related types to int64_t if long long is supported
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 edited a comment on pull request #4193:
URL: https://github.com/apache/incubator-nuttx/pull/4193#issuecomment-884291831
> Sent from my GalaxyPOSIX provides off64_t and a whole set of large file tile APIs for this. Off_t is traditionally 32 bits
opengroup doesn't define off64_t and lseek64 related API. What's opengroup expect is:
1. typedef off_t to 32bit, the file size limit to 4GB
2. typedef off_t to 64bit, the file size has no limitation
since opengroup only care about the source code compatibility, but not the binary compatibility. On the other hand, it's a hard requirement to keep the binary compatibility for all popular OS. So, these OS have to keep off_t as 32bit and add off64_t/lseek64.
> > @patacongo I see https://www.opengroup.org/platform/lfs.html, but where is the API list you mention?
>
> I am not an LFS expert, but the Wikipedia article kind of explains things: https://en.wikipedia.org/wiki/Large-file_support . Basically, (1) a compiler option turned large file support on or off (selecting the 32-bit or 64-bit APIs) and (2) no one cares any more since 64-bit machines rule the desktop.
>
> Linux has O_LARGEFILE that can select when a file is opened. https://man7.org/linux/man-pages/man2/open.2.html
Since NuttX never claim the binary compatibility as the goal( and is impossible from the implementation point since many Kconfig will change the key struct size), typedef off_t to int64_t is an clean and compliant implementation. The xxx64 macro just make libc more compatible with popular desktop OS:
https://linux.die.net/man/3/lseek64
http://www.qnx.com/developers/docs/7.0.0/#com.qnx.doc.neutrino.lib_ref/topic/l/lseek.html
> If off_t changes from anything but 32-bits, we also need to fix all file systems that expect off_t to be 32-bits in the binary meta-data of the file. These all need to be checked (and maybe others):
>
> ```
> fs/fat/
> fs/nxffs/
> fs/smartfs/
> fs/spiffs/
> fs/tmpfs/
> ```
>
As I mention in the summary:
1. If the file system support 64bit file natively, we need review it's code to change all 32bit type to 64bit
2. If the file system doesn't support 64bit file, we don't need change anything here since it's impossible to generate a file > 4GB.
> And possibly data structures in:
>
> ```
> include/nuttx/fs/dirent.h
> include/nuttx/fs/loop.h
> include/nuttx/fs/smart.h
> include/nuttx/fs/userfs.h
> ```
>
> And this would need to be kept in sync:
>
> ```
> include/nuttx/fs/hostfs.h:typedef int32_t nuttx_off_t;
> ```
I will review the general file sysem(e.g. hostfs and userfs) and update if needed.
> > a compiler option turned large file support on or off (selecting the 32-bit or 64-bit APIs)
>
> You can see the old uClibc LFS support built in (or not) here: https://git.uclibc.org/uClibc/tree/libc/stdio/Makefile.in#n22
This design is to keep the binary compatibility, it isn't the goal of NuttX. But anyway, I add a new option FS_LARGEFILE which is similar to UCLIBC_HAS_LFS.
> Another issue is that some of the older file system APIs do not use off_t but instead `long int` to represent file position. For example:
>
> ```
> stdio.h:long ftell(FAR FILE *stream);
> stdio.h:off_t ftello(FAR FILE *stream);
> ```
>
> Also:
>
> ```
> stdio.h:int fseek(FAR FILE *stream, long int offset, int whence);
> stdio.h:int fseeko(FAR FILE *stream, off_t offset, int whence);
> ```
Yes, this is the design flaw and why the standard add ftello/fseeko, please see the below link:
https://unix.org/version2/whatsnew/lfs20mar.html
--
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] davids5 commented on pull request #4193: fs: Change off_t and related types to int64_t if long long is supported
Posted by GitBox <gi...@apache.org>.
davids5 commented on pull request #4193:
URL: https://github.com/apache/incubator-nuttx/pull/4193#issuecomment-885718062
@xiaoxiang781216 Thnek you for all the work and problem solving. I can test this on Monday.
Please have one APP PR with all the commits that need to support this PR. Please this PR + the APPs PR pass your local CI.
Also rebase on master and list the deltas in size from master to this PR w/wo the feature added.
I can not test the Zilog build, But I will run it on PX4 CI and HW CI.
--
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 #4193: fs: Change off_t and related types to int64_t if long long is supported
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #4193:
URL: https://github.com/apache/incubator-nuttx/pull/4193#issuecomment-885603354
> @xiaoxiang781216 please make this PR and all the PR's in apps draft until you have a 100% working acceptable solution.
The issue mentioned by @patacongo is fixed, 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] davids5 commented on pull request #4193: fs: Change off_t and related types to int64_t if long long is supported
Posted by GitBox <gi...@apache.org>.
davids5 commented on pull request #4193:
URL: https://github.com/apache/incubator-nuttx/pull/4193#issuecomment-884149246
@patacongo I see https://www.opengroup.org/platform/lfs.html, but where is the API list you mention?
--
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 edited a comment on pull request #4193: fs: Change off_t and related types to int64_t if long long is supported
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 edited a comment on pull request #4193:
URL: https://github.com/apache/incubator-nuttx/pull/4193#issuecomment-884984681
> > > It should be done and fully tested on real HW before it comes in to master.
> > > Event the printf change will increase code size with FS_LARGEFILE off.
>
> > Why? I don't see any reason that the code size if FS_LARGEFILE off. Could you explain more?
>
> 1953165 - 1953157 = 8
>
> This is about code space and useless bloat. If it is Off it need to cost 0
The difference you identify is due to the cast in syslog, I can add #if/#else/#endif in the code, but all these places are the debugging log, do you think it's worth to make the code like spaghetti to save several bytes(especially if we consider that the debuging log already consume much more memory than the production code).
--
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] patacongo commented on pull request #4193: fs: Change off_t and related types to int64_t if long long is supported
Posted by GitBox <gi...@apache.org>.
patacongo commented on pull request #4193:
URL: https://github.com/apache/incubator-nuttx/pull/4193#issuecomment-884189871
>
>
> @patacongo I see https://www.opengroup.org/platform/lfs.html, but where is the API list you mention?
I am not an LFS expert, but the Wikipedia article kind of explains things: https://en.wikipedia.org/wiki/Large-file_support . Basically, (1) a compiler option turned large file support on or off (selecting the 32-bit or 64-bit APIs) and (2) no one cares any more since 64-bit machines rule the desktop.
Linux has O_LARGEFILE that can select when a file is opened. https://man7.org/linux/man-pages/man2/open.2.html
--
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 #4193: fs: Change off_t and related types to int64_t if long long is supported
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #4193:
URL: https://github.com/apache/incubator-nuttx/pull/4193#issuecomment-889287263
Sure.
--
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] davids5 merged pull request #4193: fs: Change off_t and related types to int64_t if long long is supported
Posted by GitBox <gi...@apache.org>.
davids5 merged pull request #4193:
URL: https://github.com/apache/incubator-nuttx/pull/4193
--
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] davids5 commented on a change in pull request #4193: fs: Change off_t and related types to int64_t if long long is supported
Posted by GitBox <gi...@apache.org>.
davids5 commented on a change in pull request #4193:
URL: https://github.com/apache/incubator-nuttx/pull/4193#discussion_r673899009
##########
File path: include/dirent.h
##########
@@ -86,6 +86,14 @@
#define DT_LNK DTYPE_LINK
#define DT_SOCK DTYPE_SOCK
+#define dirent64 dirent
+#define readdir64 readdir
+#define readdir64_r readdir_r
+#define scandir64 scandir
+#define alphasort64 alphasort
+#define versionsort64 versionsort
+#define getdents64 getdents
+
Review comment:
Please point to where this is defined in pubs.opengroup.org
If it is shims for linux maybe we should consider a different approach.
Should this all be behind the Kconfig to get an error if config is wrong?
--
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 #4193: fs: Change off_t and related types to int64_t if long long is supported
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #4193:
URL: https://github.com/apache/incubator-nuttx/pull/4193#issuecomment-884065755
Ok, I will add a new Kconfig.
--
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] davids5 edited a comment on pull request #4193: fs: Change off_t and related types to int64_t if long long is supported
Posted by GitBox <gi...@apache.org>.
davids5 edited a comment on pull request #4193:
URL: https://github.com/apache/incubator-nuttx/pull/4193#issuecomment-891253623
@xiaoxiang781216 I am not understanding the change. I thought the left side was the "correct way" What was the reason for these:
Did you back them out to save on the code space? Would you explain how this works when on a 32 bit machine with large support? Are we going to truncate the offsets in debug?
Would something like [this](https://patchwork.ozlabs.org/project/linux-mtd/patch/1486831412-30409-2-git-send-email-torfl@t-online.de/) be better all the way around?
--
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] patacongo edited a comment on pull request #4193: fs: Change off_t and related types to int64_t if long long is supported
Posted by GitBox <gi...@apache.org>.
patacongo edited a comment on pull request #4193:
URL: https://github.com/apache/incubator-nuttx/pull/4193#issuecomment-884333212
Another thing that we will have to be careful with is in reading and writing 64-bit values. On a 32-bit ARMv7-M CPU, 64-bit access are not atomic. The CPU can be interrupted or the task can be suspended between each 32-bit access and this may result in a corrupted access.
Every 64-bit access must be protected so that it completes without modification at the interrupt level or by other tasks. sched_lock() is probably sufficient in most cases. Inside of file system logic, re-entrancy protections may also be sufficient (?)
--
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 change in pull request #4193: fs: Change off_t and related types to int64_t if long long is supported
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #4193:
URL: https://github.com/apache/incubator-nuttx/pull/4193#discussion_r675760900
##########
File path: include/sys/types.h
##########
@@ -175,6 +183,16 @@ typedef int wint_t;
typedef int wctype_t;
+#if defined(CONFIG_FS_LARGEFILE) && defined(CONFIG_HAVE_LONG_LONG)
+/* Large file versions */
+
+typedef uint64_t fsblkcnt_t;
+typedef uint64_t fsfilcnt_t;
+
+typedef uint64_t blkcnt_t;
+typedef int64_t off_t;
Review comment:
fsfilcnt_t may not, but block count is related to large file support. For example, my laptop buyed two years ago has a SSD with 2TB, if the sector is 512B, 32bit block count can't represent my old SSD correctly. Other libc has the similar change for both fsfilcnt_t and blkcnt_t:
https://github.com/bminor/musl/blob/cfdfd5ea3ce14c6abf7fb22a531f3d99518b5a1b/include/sys/types.h#L74-L80
https://github.com/bminor/glibc/blob/595c22ecd8e87a27fd19270ed30fdbae9ad25426/posix/bits/types.h#L179-L189
--
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] patacongo commented on pull request #4193: fs: Change off_t and related types to int64_t if long long is supported
Posted by GitBox <gi...@apache.org>.
patacongo commented on pull request #4193:
URL: https://github.com/apache/incubator-nuttx/pull/4193#issuecomment-884333212
Another thing that we will have to be careful with is in reading and writing 64-bit values. On a 32-bit ARMv7-M CPU, 64-bit access are not atomic. The CPU can be interrupted or the task can be suspended between each 32-bit access and this may result in a corrupted access.
Every 64-bit access must be protected so that it completes without modification at the interrupt level or by other tasks. sched_lock() is probably sufficient in most cases.
--
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] patacongo commented on a change in pull request #4193: fs: Change off_t and related types to int64_t if long long is supported
Posted by GitBox <gi...@apache.org>.
patacongo commented on a change in pull request #4193:
URL: https://github.com/apache/incubator-nuttx/pull/4193#discussion_r675263956
##########
File path: include/nuttx/fs/hostfs.h
##########
@@ -106,8 +106,13 @@ typedef int16_t nuttx_uid_t;
typedef uint16_t nuttx_dev_t;
typedef uint16_t nuttx_ino_t;
typedef uint16_t nuttx_nlink_t;
+#ifdef CONFIG_FS_LARGEFILE
+typedef int64_t nuttx_off_t;
Review comment:
Requires CONFIG_LONG_LONG. This will break all ZiLOG platforms since the ZiLOG compiler does not support int64_t types.
--
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 #4193: fs: Change off_t and related types to int64_t if long long is supported
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #4193:
URL: https://github.com/apache/incubator-nuttx/pull/4193#issuecomment-888939737
@davids5 how about you test?
--
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] davids5 commented on pull request #4193: fs: Change off_t and related types to int64_t if long long is supported
Posted by GitBox <gi...@apache.org>.
davids5 commented on pull request #4193:
URL: https://github.com/apache/incubator-nuttx/pull/4193#issuecomment-885583258
@xiaoxiang781216 please make this PR and all the PR's in apps draft until you have a 100% working acceptable solution.
--
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] davids5 commented on pull request #4193: fs: Change off_t and related types to int64_t if long long is supported
Posted by GitBox <gi...@apache.org>.
davids5 commented on pull request #4193:
URL: https://github.com/apache/incubator-nuttx/pull/4193#issuecomment-891785986
@xiaoxiang781216 - What about all the apps changes that were done and committed already? Would you do a app's PR that propagates the format changes to all those as well?
--
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] davids5 edited a comment on pull request #4193: fs: Change off_t and related types to int64_t if long long is supported
Posted by GitBox <gi...@apache.org>.
davids5 edited a comment on pull request #4193:
URL: https://github.com/apache/incubator-nuttx/pull/4193#issuecomment-885718062
@xiaoxiang781216 Thank you for all the work and problem solving. I can test this on Monday.
Please have one APP PR with all the commits that need to support this PR. Please have this PR + the APPs PR pass your local CI.
Also rebase on master and list the deltas in size from master to this PR w/wo the feature added.
I can not test the Zilog build, But I will run it on PX4 CI and HW CI.
--
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 edited a comment on pull request #4193: fs: Change off_t and related types to int64_t if long long is supported
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 edited a comment on pull request #4193:
URL: https://github.com/apache/incubator-nuttx/pull/4193#issuecomment-884373965
Here is the size difference on stm32f4discovery:nsh:
```
arm-none-eabi-size nuttx.lfs
text data bss dec hex filename
70332 108 1588 72028 1195c nuttx.lfs
arm-none-eabi-size nuttx
text data bss dec hex filename
69888 108 1588 71584 117a0 nuttx
```
The LFS version increase 444B(0.6%). @davids5 @patacongo do you think FS_LARGEFILE is still really needed?
If it the last 444 byes of free flash for a unused feature yes it needs to be switchable.
What are the down size of havig the Kconfig?
--
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] patacongo commented on a change in pull request #4193: fs: Change off_t and related types to int64_t if long long is supported
Posted by GitBox <gi...@apache.org>.
patacongo commented on a change in pull request #4193:
URL: https://github.com/apache/incubator-nuttx/pull/4193#discussion_r675263956
##########
File path: include/nuttx/fs/hostfs.h
##########
@@ -106,8 +106,13 @@ typedef int16_t nuttx_uid_t;
typedef uint16_t nuttx_dev_t;
typedef uint16_t nuttx_ino_t;
typedef uint16_t nuttx_nlink_t;
+#ifdef CONFIG_FS_LARGEFILE
+typedef int64_t nuttx_off_t;
Review comment:
Requires CONFIG_LONG_LONG. This will break all ZiLOG platforms since the ZiLOG compiler does not support int64_t types. Similarly for other references to CONFIG_FS_LARGEFILE
--
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] davids5 commented on pull request #4193: fs: Change off_t and related types to int64_t if long long is supported
Posted by GitBox <gi...@apache.org>.
davids5 commented on pull request #4193:
URL: https://github.com/apache/incubator-nuttx/pull/4193#issuecomment-884929787
> Here is the size difference on stm32f4discovery:nsh:
@xiaoxiang781216 did you rerun this with all the current changes and debug on and off?
--
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] patacongo edited a comment on pull request #4193: fs: Change off_t and related types to int64_t if long long is supported
Posted by GitBox <gi...@apache.org>.
patacongo edited a comment on pull request #4193:
URL: https://github.com/apache/incubator-nuttx/pull/4193#issuecomment-884381207
I think that there is another issue with system calls on 32-bit platforms. Passing a 64-bit value will require two registers instead of one, so it will be necessary to use a different system call to pass the values, won't it? All of the registers passed in the system call are represented by uintptr_t which is 32-bits on ARMv7-M. An int32_t cannot be represented as a single uintptr_t and will have to be separated into two 32-bit values in the proxy and restored to a single 64-bit value in the stub.
Also, do the 64-bit values need to lie on aligned, even registers?
--
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 #4193: fs: Change off_t and related types to int64_t if long long is supported
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #4193:
URL: https://github.com/apache/incubator-nuttx/pull/4193#issuecomment-884373965
Here is the size difference on stm32f4discovery:nsh:
```
arm-none-eabi-size nuttx.lfs
text data bss dec hex filename
70332 108 1588 72028 1195c nuttx.lfs
arm-none-eabi-size nuttx
text data bss dec hex filename
69888 108 1588 71584 117a0 nuttx
```
The LFS version increase 444B(0.6%). @davids5 @patacongo do you think FS_LARGEFILE is still really needed?
--
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 #4193: fs: Change off_t and related types to int64_t if long long is supported
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #4193:
URL: https://github.com/apache/incubator-nuttx/pull/4193#issuecomment-884883213
> @xiaoxiang781216 - Did you see my request to do this work on a branch? The type of change could be like the change to the File FS flags, that will have a lot of clean up. It would be better not to trickle this in piecemeal.
As I already explain before, this is VFS layer change which isn't related to the individual file system. If one particular file system doesn't support the file size large than 4GB, we shouldn't modify the file system directly to support this feature. Instead, you can ask the file system designer to add the >4GB support. For example, Microsoft never plan to support >4GB on FAT, but develop a new file system for it:
https://en.wikipedia.org/wiki/ExFAT
So, it expect that both 32bit and 64bit file system coexist on NuttX(Linux too) for ever. Here is a old summary how this feature support on Linux:
https://users.suse.com/~aj/linux_lfs.html
> It should be done and fully tested on real HW before it comes in to master.
> Event the printf change will increase code size with FS_LARGEFILE off.
Why? I don't see any reason that the code size if FS_LARGEFILE off. Could you explain more?
> So lets get the real impact of this change on code size and make it really work 1005 on all the FS, before it come in to master.
FS should work as before with FS_LARGEFILE==y when the file size is smaller than 4GB from theory, but we need test anyway.
So, this patch is completed from the VFS layer and all file system should work as before when file size is smaller than 4GB. The next action is reviewing each individual file system and implement the large file support if it support natively.
I don't commit to do this work for all file system supported by NuttX. My interesting is littlefs, hostfs, userfs and maybe nfs.
But, anyway this patch doesn't make the situation worse, why not merge to mainline and let user try and feedback.
--
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 #4193: fs: Change off_t and related types to int64_t if long long is supported
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #4193:
URL: https://github.com/apache/incubator-nuttx/pull/4193#issuecomment-885033582
> > > > > It should be done and fully tested on real HW before it comes in to master.
> > > > > Event the printf change will increase code size with FS_LARGEFILE off.
> > >
> > >
> > > > Why? I don't see any reason that the code size if FS_LARGEFILE off. Could you explain more?
> > >
> > >
> > > 1953165 - 1953157 = 8
> > > This is about code space and useless bloat. If it is Off it need to cost 0
> >
> >
> > The difference you identify is due to the cast in syslog, I can add #if/#else/#endif in the code, but all these places are the debugging log, do you think it's worth to make the code like spaghetti to save several bytes(especially if we consider that the debuging log already consume much more memory than the production code).
>
> > > the effect can be seen in this PR it cost 8 bytes (yes it is a debug path, but it still need to fit when needed)
>
> Of course is it the cast. I and already said it was the debug path.
>
> No we do not want any more if def rash. (Maybe some PRI and type magic?) The is the point is. What it the total cost of this change in resources when it is OFF?
>
> Will debug builds still fit on constrained SoC that do not need 64Bit offsets for their files systems?
>
Ok, done, please try it.
> The changes from 10.0-10.1+ has already added 3.7K if the offset is referenced 25 times in debug code that that is another 1/4 K.
>
As @acassis suggest, it's better to integrate some basic size check to CI.
> All I am asking you do is not kill the small of Nuttx. Please see what you can do here.
--
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 change in pull request #4193: fs: Change off_t and related types to int64_t if long long is supported
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #4193:
URL: https://github.com/apache/incubator-nuttx/pull/4193#discussion_r682244580
##########
File path: arch/arm/src/cxd56xx/cxd56_sfc.c
##########
@@ -82,7 +82,8 @@ static int cxd56_erase(FAR struct mtd_dev_s *dev, off_t startblock,
int ret;
size_t i;
- finfo("erase: %08lx (%u blocks)\n", startblock << PAGE_SHIFT, nblocks);
+ finfo("erase: %" PRIxOFF " (%u blocks)\n",
Review comment:
Yes, it's my intention to remove 08 since startblock isn't always 4bytes after this patch which make the prefix isn't right in 64bit case.
--
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] davids5 commented on pull request #4193: fs: Change off_t and related types to int64_t if long long is supported
Posted by GitBox <gi...@apache.org>.
davids5 commented on pull request #4193:
URL: https://github.com/apache/incubator-nuttx/pull/4193#issuecomment-884197550
@xiaoxiang781216 @patacongo - How about doing this work on a branch and put keep the option behind a Kconfig?
--
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 change in pull request #4193: fs: Change off_t and related types to int64_t if long long is supported
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #4193:
URL: https://github.com/apache/incubator-nuttx/pull/4193#discussion_r675525701
##########
File path: include/nuttx/fs/hostfs.h
##########
@@ -106,8 +106,13 @@ typedef int16_t nuttx_uid_t;
typedef uint16_t nuttx_dev_t;
typedef uint16_t nuttx_ino_t;
typedef uint16_t nuttx_nlink_t;
+#ifdef CONFIG_FS_LARGEFILE
+typedef int64_t nuttx_off_t;
Review comment:
Ok, we can check both CONFIG_FS_LARGEFILE and CONFIG_LONG_LONG to fix this issue. Please review the new version.
--
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] davids5 commented on pull request #4193: fs: Change off_t and related types to int64_t if long long is supported
Posted by GitBox <gi...@apache.org>.
davids5 commented on pull request #4193:
URL: https://github.com/apache/incubator-nuttx/pull/4193#issuecomment-888996343
@xiaoxiang781216 I am sorry for the delay. I had some pressing issues the preempted it. I will shoot for Friday. If not it will Monday then.
--
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] davids5 commented on pull request #4193: fs: Change off_t and related types to int64_t if long long is supported
Posted by GitBox <gi...@apache.org>.
davids5 commented on pull request #4193:
URL: https://github.com/apache/incubator-nuttx/pull/4193#issuecomment-884855071
@xiaoxiang781216 - Did you see my request to do this work on a branch? The type of change could be like the change to the File FS flags, that will have a lot of clean up. It would be better not to trickle this in piecemeal. It should be done and fully tested on real HW before it comes in to master. Event the printf change will increase code size with FS_LARGEFILE off. So lets get the real impact of this change on code size and make it really work 1005 on all the FS, before it come in to master.
--
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 edited a comment on pull request #4193: fs: Change off_t and related types to int64_t if long long is supported
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 edited a comment on pull request #4193:
URL: https://github.com/apache/incubator-nuttx/pull/4193#issuecomment-885603354
> @xiaoxiang781216 please make this PR and all the PR's in apps draft until you have a 100% working acceptable solution.
The issue mentioned by @patacongo is fixed, please take a look.
BTW, https://github.com/apache/incubator-nuttx-apps/pull/812 need merge first to fix the compiler warning.
--
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 change in pull request #4193: fs: Change off_t and related types to int64_t if long long is supported
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #4193:
URL: https://github.com/apache/incubator-nuttx/pull/4193#discussion_r675285099
##########
File path: include/nuttx/fs/hostfs.h
##########
@@ -106,8 +106,13 @@ typedef int16_t nuttx_uid_t;
typedef uint16_t nuttx_dev_t;
typedef uint16_t nuttx_ino_t;
typedef uint16_t nuttx_nlink_t;
+#ifdef CONFIG_FS_LARGEFILE
+typedef int64_t nuttx_off_t;
Review comment:
Yes, I found this issue before, but it's hard to let FS_LARGEFILE depends on LONG_LONG since LONG_LONG isn't a Kconfig option, but auto define in include/nuttx/compiler.h:
https://github.com/apache/incubator-nuttx/blob/master/include/nuttx/compiler.h#L169
Do you have some better method to resolve this issue?
--
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 change in pull request #4193: fs: Change off_t and related types to int64_t if long long is supported
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #4193:
URL: https://github.com/apache/incubator-nuttx/pull/4193#discussion_r675760900
##########
File path: include/sys/types.h
##########
@@ -175,6 +183,16 @@ typedef int wint_t;
typedef int wctype_t;
+#if defined(CONFIG_FS_LARGEFILE) && defined(CONFIG_HAVE_LONG_LONG)
+/* Large file versions */
+
+typedef uint64_t fsblkcnt_t;
+typedef uint64_t fsfilcnt_t;
+
+typedef uint64_t blkcnt_t;
+typedef int64_t off_t;
Review comment:
fsfilcnt_t may not, but block count is related to large file support. For example, my laptop buyed two years ago has a SSD with 2TB, if the sector is 512B, 32bit block count can't represent my old SSD correctly. Other libc has the similar change:
https://github.com/bminor/musl/blob/cfdfd5ea3ce14c6abf7fb22a531f3d99518b5a1b/include/sys/types.h#L74-L80
https://github.com/bminor/glibc/blob/595c22ecd8e87a27fd19270ed30fdbae9ad25426/posix/bits/types.h#L179-L189
--
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 #4193: fs: Change off_t and related types to int64_t if long long is supported
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #4193:
URL: https://github.com/apache/incubator-nuttx/pull/4193#issuecomment-884984681
> > > It should be done and fully tested on real HW before it comes in to master.
> > > Event the printf change will increase code size with FS_LARGEFILE off.
>
> > Why? I don't see any reason that the code size if FS_LARGEFILE off. Could you explain more?
>
> 1953165 - 1953157 = 8
>
> This is about code space and useless bloat. If it is Off it need to cost 0
The difference you identify is due to the cast in syslog, I can add #if/#else/#endif in the code, but all these places are the debugging log, do you think it's worth to make the code like spaghetti to save several bytes(especially the debuging log already consume much more memory than the production code).
--
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] davids5 merged pull request #4193: fs: Change off_t and related types to int64_t if long long is supported
Posted by GitBox <gi...@apache.org>.
davids5 merged pull request #4193:
URL: https://github.com/apache/incubator-nuttx/pull/4193
--
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] davids5 commented on pull request #4193: fs: Change off_t and related types to int64_t if long long is supported
Posted by GitBox <gi...@apache.org>.
davids5 commented on pull request #4193:
URL: https://github.com/apache/incubator-nuttx/pull/4193#issuecomment-891253623
@xiaoxiang781216 I am not understanding the change. I thought the left size was the "correct way" What was the reason for these:
Did you back them out to save on the code space? Would you explain how this works when on a 32 bit machine with large support? Are we going to truncate the offsets in debug?
Would something like [this](https://patchwork.ozlabs.org/project/linux-mtd/patch/1486831412-30409-2-git-send-email-torfl@t-online.de/) be better all the way around?
--
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] davids5 commented on pull request #4193: fs: Change off_t and related types to int64_t if long long is supported
Posted by GitBox <gi...@apache.org>.
davids5 commented on pull request #4193:
URL: https://github.com/apache/incubator-nuttx/pull/4193#issuecomment-885005271
> > > > It should be done and fully tested on real HW before it comes in to master.
> > > > Event the printf change will increase code size with FS_LARGEFILE off.
> >
> >
> > > Why? I don't see any reason that the code size if FS_LARGEFILE off. Could you explain more?
> >
> >
> > 1953165 - 1953157 = 8
> > This is about code space and useless bloat. If it is Off it need to cost 0
>
> The difference you identify is due to the cast in syslog, I can add #if/#else/#endif in the code, but all these places are the debugging log, do you think it's worth to make the code like spaghetti to save several bytes(especially if we consider that the debuging log already consume much more memory than the production code).
>> the effect can be seen in this PR it cost 8 bytes (yes it is a debug path, but it still need to fit when needed)
Of course is it the cast. I and already said it was the debug path.
No we do not want any more if def rash. (Maybe some PRI and type magic?) The is the point is. What it the total cost of this change in resources when it is OFF?
Will debug builds still fit on constrained SoC that do not need 64Bit offsets for their files systems?
The changes from 10.0-10.1+ has already added 3.7K if the offset is referenced 25 times in debug code that that is another 1/4 K.
All I am asking you do is not kill the small of Nuttx. Please see what you can do here.
--
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] davids5 commented on pull request #4193: fs: Change off_t and related types to int64_t if long long is supported
Posted by GitBox <gi...@apache.org>.
davids5 commented on pull request #4193:
URL: https://github.com/apache/incubator-nuttx/pull/4193#issuecomment-891194513
@xiaoxiang781216 - I have tested this and it is working well with and without Large File Support turned on.
The Knob was very worthwhile.!
master + these commits
LFS not enabled compared to enabled
```
VM SIZE
--------------
+0.2% +3.33Ki .text
+41% +176 ftl_write
+54% +152 fat_checkbootrecord
+34% +116 fat_seek
+26% +112 fat_write
+39% +96 fat_dirextend
+29% +80 cromfs_read
+21% +78 fat_allocatedirentry
+28% +70 fat_putcluster
+31% +64 mtd_partition
+97% +62 part_read
+97% +62 part_write
[NEW] +60 cdev::cdev_seek(file*, long long, int)
+30% +58 fat_getcluster
+5.9% +56 fat_finddirentry
+6.4% +56 hardfault_commit.constprop.0
+11% +56 px4::logger::util::check_free_space(char const*, long, void*&, int&)
+56% +54 fat_nextdirentry
+104% +48 part_erase
+55% +48 romfs_seek
+58% +44 stm32_bbsram_seek
+35% +42 fat_computefreeclusters
+12% +42 fat_read
+105% +42 part_bread
+105% +42 part_bwrite
+50% +40 bch_seek
+13% +40 fat_mount
+28% +40 fat_truncate
+59% +38 stm32_bbsram_read
+22% +36 at24c_bwrite
+5.7% +36 bl_update_main
+62% +36 fat_currentsector
+25% +36 fat_extendchain
+27% +34 fat_dirshrink
+8.9% +34 fat_mkdir
+34% +34 stm32_bbsram_write
+19% +32 _file_restart(dm_reset_reason)
+27% +32 fat_freedirentry
+114% +32 ramtron_bread
+35% +30 fat_fscacheflush
+16% +28 at24c_bread
+27% +28 fat_rewinddir
+44% +28 file_seek
+1.8% +28 proc_read
+14% +26 inode_stat
+30% +24 mount_read
+8.3% +24 mtd_rwtest(mtd_instance_s const&)
+55% +24 procfs_memcpy
+41% +24 rd_write
+16% +22 fat_dup
+17% +22 fat_stat_file
+44% +22 rd_read
+13% +20 _file_clear(dm_item_t)
+24% +20 blocks_entry
+2.0% +20 fat_putlfname
+26% +20 ftell
+77% +20 lseek
+67% +20 nx_seek
+24% +20 romfs_opendir
+15% +18 fat_findsfnentry
+14% +18 fat_opendir
+64% +18 gpio_seek
+16% +18 part_ioctl
+18% +18 romfs_stat_common
+6.3% +16 MavlinkStreamStorageInformation::send()
+62% +16 fat_cluster2sector
+26% +16 fat_ffcacheread
+38% +16 fat_fscacheread
+6.4% +16 files_allocate
+13% +16 romfs_readdir
+14% +16 uptime_read
+5.0% +16 usage_entry
+17% +16 version_read
+4.3% +14 fat_open
+6.4% +14 readdir
+12% +12 MavlinkFTP::_workWrite(MavlinkFTP::PayloadHeader*)
+7.9% +12 _file_read(dm_item_t, unsigned int, void*, unsigned int)
+27% +12 fat_ffcacheflush
+6.8% +12 fat_readdir
+27% +12 gpio_read
+4.5% +12 iobinfo_read
+2.8% +12 meminfo_read
+6.7% +12 part_procfs_read
+3.3% +12 procfs_readdir
+4.5% +12 uavcan_posix::BasicFileServerBackend::read(uavcan::Array<uavcan::IntegerSpec<8u, (uavcan::Signedness)0, (uavcan::CastMode)0>, (uavcan::ArrayMode)1, 200u> const&, unsigned long long, unsigned char*, unsigned short&)
+6.8% +10 bson_encoder_fini(bson_encoder_s*)
[NEW] +10 cdev::CDev::seek(file*, long long, int)
+4.7% +10 fat_remove
+4.2% +10 file_vopen
+3.8% +10 romfs_finddirentry
+12% +10 romfs_statfs
+6.9% +8 MavlinkFTP::_workRead(MavlinkFTP::PayloadHeader*)
+6.7% +8 PreFlightCheck::sdcardCheck(void**, bool&, bool)
+0.0% +8 [section .text]
+5.6% +8 _file_write(dm_item_t, unsigned int, dm_persitence_t, void const*, unsigned int)
+14% +8 bch_read
+11% +8 bch_write
+50% +8 binfs_fstat
+17% +8 binfs_stat
+17% +8 cromfs_statfs
+1.0% +8 expression
+1.8% +8 fat_dirname2path
+11% +8 fat_dirtruncate
+21% +8 fat_findalias
+9.1% +8 fat_statfs
+25% +8 netprocfs_read
+8.3% +8 netprocfs_stat
+50% +8 part_procfs_stat
+29% +8 procfs_statfs
+0.8% +8 px4_mtd_config
+4.3% +8 romfs_read
+2.2% +8 update(char const*, char const*)
+19% +6 bchlib_flushsector
+3.1% +6 bchlib_read
+13% +6 bchlib_readsector
+2.7% +6 bchlib_write
+23% +6 binfs_statfs
+16% +6 fat_ffcacheinvalidate
+14% +6 file_close
+4.1% +6 file_dup2
+10.0% +6 fseek
+3.4% +6 ipcfg_read_binary_ipv4
+2.8% +6 opendir
+2.9% +4 MavlinkFTP::_workOpen(MavlinkFTP::PayloadHeader*, int)
+1.3% +4 MavlinkFTP::_workTruncateFile(MavlinkFTP::PayloadHeader*)
+1.1% +4 MavlinkFTP::send()
+1.9% +4 PX4IO_Uploader::program(unsigned int)
+1.3% +4 PX4IO_Uploader::verify_rev2(unsigned int)
+9.5% +4 cromfs_fstat
+5.9% +4 cromfs_stat
+5.0% +4 fat_fstat
+11% +4 fat_hwread
+11% +4 fat_hwwrite
+1.5% +4 fat_rename
+3.6% +4 fat_updatefsinfo
+6.7% +4 file_dup
+4.0% +4 files_duplist
+50% +4 gpio_open
+0.7% +4 hardfault_write
+6.2% +4 nx_vopen
+2.7% +4 param_import_internal(int, bool)
+4.8% +4 proc_readdir
+10.0% +4 romfs_rewinddir
+11% +4 sockfd_allocate
+5.0% +4 write_intterupt_stack.constprop.0
+5.0% +4 write_user_stack.constprop.0
+4.5% +2 binfs_readdir
+1.0% +2 cromfs_readdir
+3.6% +2 fat_checkmount
+5.6% +2 fat_nfreeclusters
+1.0% +2 fat_sync
+2.1% +2 fs_getfilep
+6.2% +2 ftl_geometry
+2.1% +2 mmcsd_geometry
+2.2% +2 netprocfs_readdir
+1.7% +2 nx_close
+1.5% +2 nx_dup2
+0.9% +2 ramtron_bwrite
+5.9% +2 rd_geometry
+2.9% +2 romfs_checkmount
+1.0% +2 romfs_open
-2.8% -2 fat_removechain
-1.7% -2 ramtron_read
-2.1% -2 romfs_hwread
-12.5% -4 ftl_read
-3.1% -4 hardfault_increment_reboot
[DEL] -6 cdev::CDev::seek(file*, long, int)
[DEL] -56 cdev::cdev_seek(file*, long, int)
+0.2% +3.33Ki TOTAL
```
Master @8660572a3c69e0e2ca24c3cabeaaaae3372a6960 compared to these commits enabled
```
VM SIZE
--------------
+0.2% +3.29Ki .text
[NEW] +216 fnmatch_one
+41% +176 ftl_write
+54% +152 fat_checkbootrecord
[NEW] +140 default_gpios.9884
[NEW] +140 gpio.11297
[NEW] +120 commands.23401
+34% +116 fat_seek
+26% +112 fat_write
[NEW] +108 work_thread
[NEW] +108 work_thread_create.constprop.0
+39% +96 fat_dirextend
+29% +80 cromfs_read
+143% +80 work_queue
+21% +78 fat_allocatedirentry
+28% +70 fat_putcluster
[NEW] +66 fnmatch
+31% +64 mtd_partition
+97% +62 part_read
+97% +62 part_write
[NEW] +60 cdev::cdev_seek(file*, long long, int)
+30% +58 fat_getcluster
+1.8% +56 [28 Others]
+5.9% +56 fat_finddirentry
+6.4% +56 hardfault_commit.constprop.0
+11% +56 px4::logger::util::check_free_space(char const*, long, void*&, int&)
+56% +54 fat_nextdirentry
[NEW] +52 hp_work_timer_expiry
[NEW] +52 lp_work_timer_expiry
[NEW] +48 page_sizes.7944
+104% +48 part_erase
+55% +48 romfs_seek
[NEW] +44 dn2o.8887
+58% +44 stm32_bbsram_seek
+35% +42 fat_computefreeclusters
+12% +42 fat_read
+105% +42 part_bread
+105% +42 part_bwrite
+50% +40 bch_seek
+13% +40 fat_mount
+28% +40 fat_truncate
+59% +38 stm32_bbsram_read
+22% +36 at24c_bwrite
+5.7% +36 bl_update_main
+62% +36 fat_currentsector
+25% +36 fat_extendchain
+27% +34 fat_dirshrink
+8.9% +34 fat_mkdir
+34% +34 stm32_bbsram_write
+19% +32 _file_restart(dm_reset_reason)
+27% +32 fat_freedirentry
+114% +32 ramtron_bread
+35% +30 fat_fscacheflush
+16% +28 at24c_bread
+27% +28 fat_rewinddir
+44% +28 file_seek
+1.8% +28 proc_read
+14% +26 inode_stat
+30% +24 mount_read
+8.3% +24 mtd_rwtest(mtd_instance_s const&)
+55% +24 procfs_memcpy
+41% +24 rd_write
+16% +22 fat_dup
+17% +22 fat_stat_file
[NEW] +22 format.8416
+44% +22 rd_read
+13% +20 _file_clear(dm_item_t)
[NEW] +20 baudrates.11002
[NEW] +20 baudrates_to_try.10178
+24% +20 blocks_entry
+2.0% +20 fat_putlfname
+26% +20 ftell
+77% +20 lseek
+67% +20 nx_seek
+24% +20 romfs_opendir
+15% +18 fat_findsfnentry
+14% +18 fat_opendir
+64% +18 gpio_seek
+16% +18 part_ioctl
+18% +18 romfs_stat_common
+6.3% +16 MavlinkStreamStorageInformation::send()
[NEW] +16 __FUNCTION__.8397
+62% +16 fat_cluster2sector
+26% +16 fat_ffcacheread
+38% +16 fat_fscacheread
+6.4% +16 files_allocate
+13% +16 romfs_readdir
[NEW] +16 sensors_channels.17663
+14% +16 uptime_read
+5.0% +16 usage_entry
+17% +16 version_read
[NEW] +14 __FUNCTION__.8035
+4.3% +14 fat_open
+6.4% +14 readdir
[NEW] +13 __FUNCTION__.8381
[NEW] +13 __FUNCTION__.8407
+12% +12 MavlinkFTP::_workWrite(MavlinkFTP::PayloadHeader*)
[NEW] +12 __FUNCTION__.8389
+7.9% +12 _file_read(dm_item_t, unsigned int, void*, unsigned int)
+27% +12 fat_ffcacheflush
+6.8% +12 fat_readdir
+27% +12 gpio_read
+4.5% +12 iobinfo_read
+2.8% +12 meminfo_read
+6.7% +12 part_procfs_read
+3.3% +12 procfs_readdir
+4.5% +12 uavcan_posix::BasicFileServerBackend::read(uavcan::Array<uavcan::IntegerSpec<8u, (uavcan::Signedness)0, (uavcan::CastMode)0>, (uavcan::ArrayMode)1, 200u> const&, unsigned long long, unsigned char*, unsigned short&)
+21% +12 work_qcancel
[NEW] +10 __FUNCTION__.8416
+6.8% +10 bson_encoder_fini(bson_encoder_s*)
[NEW] +10 cdev::CDev::seek(file*, long long, int)
+4.7% +10 fat_remove
+4.2% +10 file_vopen
+3.8% +10 romfs_finddirentry
+12% +10 romfs_statfs
+6.9% +8 MavlinkFTP::_workRead(MavlinkFTP::PayloadHeader*)
+6.7% +8 PreFlightCheck::sdcardCheck(void**, bool&, bool)
+5.6% +8 _file_write(dm_item_t, unsigned int, dm_persitence_t, void const*, unsigned int)
+14% +8 bch_read
+11% +8 bch_write
+50% +8 binfs_fstat
+9.1% +8 binfs_operations
+17% +8 binfs_stat
+9.1% +8 cromfs_operations
+17% +8 cromfs_statfs
+1.0% +8 expression
+1.8% +8 fat_dirname2path
+11% +8 fat_dirtruncate
+21% +8 fat_findalias
+9.1% +8 fat_operations
+9.1% +8 fat_statfs
[NEW] +8 min_available.14907
+25% +8 netprocfs_read
+8.3% +8 netprocfs_stat
+50% +8 part_procfs_stat
+9.1% +8 procfs_operations
+29% +8 procfs_statfs
+0.8% +8 px4_mtd_config
+9.1% +8 romfs_operations
+4.3% +8 romfs_read
+2.2% +8 update(char const*, char const*)
+19% +6 bchlib_flushsector
+3.1% +6 bchlib_read
+13% +6 bchlib_readsector
+2.7% +6 bchlib_write
+23% +6 binfs_statfs
+16% +6 fat_ffcacheinvalidate
+14% +6 file_close
+4.1% +6 file_dup2
+10.0% +6 fseek
+3.4% +6 ipcfg_read_binary_ipv4
+2.8% +6 opendir
[NEW] +5 labels.8590
+2.9% +4 MavlinkFTP::_workOpen(MavlinkFTP::PayloadHeader*, int)
+1.3% +4 MavlinkFTP::_workTruncateFile(MavlinkFTP::PayloadHeader*)
+1.1% +4 MavlinkFTP::send()
+1.9% +4 PX4IO_Uploader::program(unsigned int)
+1.3% +4 PX4IO_Uploader::verify_rev2(unsigned int)
+0.0% +4 [section .text]
+9.5% +4 cromfs_fstat
+5.9% +4 cromfs_stat
+5.0% +4 fat_fstat
+11% +4 fat_hwread
+11% +4 fat_hwwrite
+1.5% +4 fat_rename
+3.6% +4 fat_updatefsinfo
+6.7% +4 file_dup
+18% +4 file_socket
+4.0% +4 files_duplist
+50% +4 gpio_open
-12.5% -4 ftl_read
-0.0% -4 g_cromfs_image
-3.1% -4 hardfault_increment_reboot
[DEL] -5 labels.8564
[DEL] -6 cdev::CDev::seek(file*, long, int)
[DEL] -8 min_available.14884
-0.9% -8 mmcsd_probe
[DEL] -10 __FUNCTION__.8393
[DEL] -12 __FUNCTION__.8366
[DEL] -13 __FUNCTION__.8358
[DEL] -13 __FUNCTION__.8384
[DEL] -14 __FUNCTION__.8012
[DEL] -16 __FUNCTION__.8374
[DEL] -16 sensors_channels.17636
[DEL] -20 baudrates.10979
[DEL] -20 baudrates_to_try.10155
[DEL] -20 work_hpthread
[DEL] -20 work_lpthread
[DEL] -22 format.8393
[DEL] -44 dn2o.8864
[DEL] -44 work_signal
[DEL] -48 page_sizes.7921
-66.7% -48 work_start_highpri
-66.7% -48 work_start_lowpri
[DEL] -56 cdev::cdev_seek(file*, long, int)
[DEL] -58 match
[DEL] -84 work_qqueue
[DEL] -120 commands.23378
[DEL] -140 default_gpios.9857
[DEL] -140 gpio.11171
[DEL] -216 match_one
[DEL] -224 work_process
+0.1% +64 .bss
[NEW] +44 tm.5737
+5.5% +24 [section .bss]
[NEW] +20 buffer.7938
[NEW] +20 channels_cache.9223
+167% +20 g_hpwork
+167% +20 g_lpwork
[NEW] +18 buffer.7997
[NEW] +8 base_time.8779
[NEW] +4 inited.6149
[NEW] +4 last_count.8780
[NEW] +4 new_channel_count.8757
[NEW] +4 new_channel_holdoff.8758
[NEW] +4 notifier_key.8193
[NEW] +4 rxdummy.8521
[NEW] +2 g_last_tcp_port.7385
[NEW] +2 g_last_udp_port.7233
[NEW] +1 initialized.5962
[NEW] +1 initialized.8270
[NEW] +1 once.8128
[NEW] +1 state.8384
[DEL] -1 initialized.5964
[DEL] -1 initialized.8247
[DEL] -1 once.8105
[DEL] -1 state.8361
[DEL] -2 g_last_tcp_port.7368
[DEL] -2 g_last_udp_port.7216
[DEL] -4 inited.6151
[DEL] -4 last_count.8757
[DEL] -4 new_channel_count.8734
[DEL] -4 new_channel_holdoff.8735
[DEL] -4 notifier_key.8169
[DEL] -4 rxdummy.8498
[DEL] -8 base_time.8756
[DEL] -18 buffer.7974
[DEL] -20 buffer.7915
[DEL] -20 channels_cache.9200
[DEL] -44 tm.5739
+0.2% +3.35Ki TOTAL
```
--
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 change in pull request #4193: fs: Change off_t and related types to int64_t if long long is supported
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #4193:
URL: https://github.com/apache/incubator-nuttx/pull/4193#discussion_r682245281
##########
File path: arch/arm/src/cxd56xx/cxd56_sfc.c
##########
@@ -101,7 +102,8 @@ static ssize_t cxd56_bread(FAR struct mtd_dev_s *dev, off_t startblock,
{
int ret;
- finfo("bread: %08lx (%u blocks)\n", startblock << PAGE_SHIFT, nblocks);
+ finfo("bread: %" PRIxOFF "(%u blocks)\n",
Review comment:
Done.
--
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] patacongo commented on pull request #4193: fs: Change off_t and related types to int64_t if long long is supported
Posted by GitBox <gi...@apache.org>.
patacongo commented on pull request #4193:
URL: https://github.com/apache/incubator-nuttx/pull/4193#issuecomment-884381207
I think that there is another issue with system calls on 32-bit platforms. Passing a 64-bit value will require two registers instead of one, so it will be necessary to use a different system call to pass the values, won't it? Also, do the 64-bit values need to lie on aligned, even registers?
--
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] gustavonihei commented on a change in pull request #4193: fs: Change off_t and related types to int64_t if long long is supported
Posted by GitBox <gi...@apache.org>.
gustavonihei commented on a change in pull request #4193:
URL: https://github.com/apache/incubator-nuttx/pull/4193#discussion_r682139399
##########
File path: arch/arm/src/cxd56xx/cxd56_sfc.c
##########
@@ -140,7 +143,7 @@ static ssize_t cxd56_read(FAR struct mtd_dev_s *dev, off_t offset,
{
int ret;
- finfo("read: %08lx (%u bytes)\n", offset, nbytes);
+ finfo("read: %" PRIxOFF "(%u bytes)\n", offset, nbytes);
Review comment:
```suggestion
finfo("read: %" PRIxOFF " (%u bytes)\n", offset, nbytes);
```
--
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