patacongo commented on a change in pull request #2861:
URL: https://github.com/apache/incubator-nuttx/pull/2861#discussion_r577618372
##########
File path: include/nuttx/fs/fs.h
##########
@@ -206,11 +206,11 @@ struct file_operations
#ifndef CONFIG_DISABLE_MOUNTPOINT
struct geometry
{
- bool geo_available; /* true: The device is available */
- bool geo_mediachanged; /* true: The media has changed since last query */
- bool geo_writeenabled; /* true: It is okay to write to this device */
- size_t geo_nsectors; /* Number of sectors on the device */
- size_t geo_sectorsize; /* Size of one sector */
+ bool geo_available; /* true: The device is available */
+ bool geo_mediachanged; /* true: The media has changed since last query */
+ bool geo_writeenabled; /* true: It is okay to write to this device */
+ uint32_t geo_nsectors; /* Number of sectors on the device */
+ uint32_t geo_sectorsize; /* Size of one sector */
Review comment:
>
>
> @patacongo, @xiaoxiang781216 Is this the correct approach?
size_t is intended to be wide enough to hold the biggest addressable object
in memory. So it varies with CPU architecture. It should have nothing to do
with the geometry of media underlying a block device.
This change will have no impact on the hordes of people using 32-bit ARMs.
For them size_t is the same as uint32_t. For people using 64-bit devices this
will shrink from 64- to 32- bits, but I don't think that is an issue; this is
not addressable memory, this size_t is determines the maximum number of sectors
that can be supported. For the eZ80, size_t is 24-bits.
For a 24-bit size_t, the use of size_t would limit the maximum size of, say,
an SD card or eMMC to 8Gb which is unnecessary. The size of addressable memory
should not affect the maximum size of the supported media.
I don't think the change is needed for geo_sectorsize which I think is 512
on all SD cards and maybe up to 4Kb on some HDDs. It is only geo_nectors where
size_t introduces a real limitation (still size_t is not good choice of types).
There may be other issues in supporting large block devices, but size_t
should not be one of them.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
