[PATCH] stat.2: fixed inode printing in example program

[PATCH] stat.2: fixed inode printing in example program

[Konstantin Bukin]

inode numbers are expected to be positive. Casting them to a signed type
may result in printing negative values. E.g. running example program on
the following file:

$ ls -li test.txt
9280843260537405888 -r--r--r-- 1 kbukin hardware 300 Jul 21 06:36 test.txt

resutls in the following output:

$ ./example test.txt
ID of containing device:  [0,480]
File type:                regular file
I-node number:            -9165900813172145728
Mode:                     100444 (octal)
Link count:               1
Ownership:                UID=2743   GID=30
Preferred I/O block size: 32768 bytes
File size:                300 bytes
Blocks allocated:         8
Last status change:       Tue Jul 21 06:36:50 2020
Last file access:         Sat Sep 12 14:13:38 2020
Last file modification:   Tue Jul 21 06:36:50 2020

Such erroneous reporting happens for inode values greater than maximum
value which can be stored in signed long. Casting does not seem to be
necessary here. Printing inode as unsigned long fixes the issue.
---
 man2/stat.2 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/man2/stat.2 b/man2/stat.2
index 7e5417480..76997bcbe 100644
--- a/man2/stat.2
+++ b/man2/stat.2
@@ -681,7 +681,7 @@ main(int argc, char *argv[])
     default:       printf("unknown?\en");                break;
     }
 
-    printf("I\-node number:            %ld\en", (long) sb.st_ino);
+    printf("I\-node number:            %lu\en", sb.st_ino);
 
     printf("Mode:                     %lo (octal)\en",
             (unsigned long) sb.st_mode);
-- 
2.17.0

Re: [PATCH] stat.2: fixed inode printing in example program

[Dmitry V. Levin]

On Sun, Sep 13, 2020 at 08:04:49AM -0700, Konstantin Bukin wrote:
> inode numbers are expected to be positive. Casting them to a signed type
> may result in printing negative values. E.g. running example program on
> the following file:
> 
> $ ls -li test.txt
> 9280843260537405888 -r--r--r-- 1 kbukin hardware 300 Jul 21 06:36 test.txt
> 
> resutls in the following output:
> 
> $ ./example test.txt
> ID of containing device:  [0,480]
> File type:                regular file
> I-node number:            -9165900813172145728
> Mode:                     100444 (octal)
> Link count:               1
> Ownership:                UID=2743   GID=30
> Preferred I/O block size: 32768 bytes
> File size:                300 bytes
> Blocks allocated:         8
> Last status change:       Tue Jul 21 06:36:50 2020
> Last file access:         Sat Sep 12 14:13:38 2020
> Last file modification:   Tue Jul 21 06:36:50 2020
> 
> Such erroneous reporting happens for inode values greater than maximum
> value which can be stored in signed long. Casting does not seem to be
> necessary here. Printing inode as unsigned long fixes the issue.
> ---
>  man2/stat.2 | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/man2/stat.2 b/man2/stat.2
> index 7e5417480..76997bcbe 100644
> --- a/man2/stat.2
> +++ b/man2/stat.2
> @@ -681,7 +681,7 @@ main(int argc, char *argv[])
>      default:       printf("unknown?\en");                break;
>      }
>  
> -    printf("I\-node number:            %ld\en", (long) sb.st_ino);
> +    printf("I\-node number:            %lu\en", sb.st_ino);

By the way, the type of st_ino is ino_t which might be larger than long,
so both the old and the new variants are not correct.


-- 
ldv

Re: [PATCH] stat.2: fixed inode printing in example program

[Konstantin Bukin]

On x86 the size of ino_t does not appear to be larger than long. It is
4 in 32-bit programs and 8 in 64-bit programs. On what platforms the
size of ino_t is larger than long?

Here is the beginning of memory taken by struct stat sb in 32-bit
example program run on my file:

(gdb) x/8xw &sb
0xffffd0d8:     0x00400080      0x00000000      0x00ca0000      0x006901c0
0xffffd0e8:     0x00008124      0x00000001      0x00000ab7      0x0000001e

The first eight bytes is st_dev. 0x006901c0 are the lower 4 bytes of
inode. I am not sure what 0x00ca0000 is. Is that just padding?

For comparison, here is the beginning of memory taken by struct stat
sb in 64-bit example programs run on the same file:

(gdb) x/8xw &sb
0x7fffffffdf30: 0x00400080      0x00000000      0x006901c0      0x80cc2dc6
0x7fffffffdf40: 0x00000001      0x00000000      0x00008124      0x00000ab7

0x006901c0 and 0x80cc2dc6 are lower and upper 4 bytes of the inode.
0x80cc2dc6006901c0 is what “ls -li” reports, but in decimal.


On Sun, Sep 13, 2020 at 8:16 AM Dmitry V. Levin <ldv@altlinux.org> wrote:
>
> On Sun, Sep 13, 2020 at 08:04:49AM -0700, Konstantin Bukin wrote:
> > inode numbers are expected to be positive. Casting them to a signed type
> > may result in printing negative values. E.g. running example program on
> > the following file:
> >
> > $ ls -li test.txt
> > 9280843260537405888 -r--r--r-- 1 kbukin hardware 300 Jul 21 06:36 test.txt
> >
> > resutls in the following output:
> >
> > $ ./example test.txt
> > ID of containing device:  [0,480]
> > File type:                regular file
> > I-node number:            -9165900813172145728
> > Mode:                     100444 (octal)
> > Link count:               1
> > Ownership:                UID=2743   GID=30
> > Preferred I/O block size: 32768 bytes
> > File size:                300 bytes
> > Blocks allocated:         8
> > Last status change:       Tue Jul 21 06:36:50 2020
> > Last file access:         Sat Sep 12 14:13:38 2020
> > Last file modification:   Tue Jul 21 06:36:50 2020
> >
> > Such erroneous reporting happens for inode values greater than maximum
> > value which can be stored in signed long. Casting does not seem to be
> > necessary here. Printing inode as unsigned long fixes the issue.
> > ---
> >  man2/stat.2 | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/man2/stat.2 b/man2/stat.2
> > index 7e5417480..76997bcbe 100644
> > --- a/man2/stat.2
> > +++ b/man2/stat.2
> > @@ -681,7 +681,7 @@ main(int argc, char *argv[])
> >      default:       printf("unknown?\en");                break;
> >      }
> >
> > -    printf("I\-node number:            %ld\en", (long) sb.st_ino);
> > +    printf("I\-node number:            %lu\en", sb.st_ino);
>
> By the way, the type of st_ino is ino_t which might be larger than long,
> so both the old and the new variants are not correct.
>
>
> --
> ldv

Re: [PATCH] stat.2: fixed inode printing in example program

[Dmitry V. Levin]

On Sun, Sep 13, 2020 at 10:24:23AM -0700, Konstantin Bukin wrote:
> On x86 the size of ino_t does not appear to be larger than long.

It depends.  For example, with glibc on all architectures ino_t is 64-bit
when the code is compiled using -D_FILE_OFFSET_BITS=64.


-- 
ldv

Re: [PATCH] stat.2: fixed inode printing in example program

[Alejandro Colomar]

Hi,

Please, see the patch I just sent:

https://lore.kernel.org/linux-man/20200913175506.576683-1-colomar.6.4.3@gmail.com/T/#u

Do you agree it solves this problem?

Cheers,

Alex

Re: [PATCH] stat.2: fixed inode printing in example program

[Konstantin Bukin]

I didn’t know _FILE_OFFSET_BITS also affects size of ino_t. Do you see
any issues printing ino_t with %lld ?


On Sun, Sep 13, 2020 at 10:38 AM Dmitry V. Levin <ldv@altlinux.org> wrote:
>
> On Sun, Sep 13, 2020 at 10:24:23AM -0700, Konstantin Bukin wrote:
> > On x86 the size of ino_t does not appear to be larger than long.
>
> It depends.  For example, with glibc on all architectures ino_t is 64-bit
> when the code is compiled using -D_FILE_OFFSET_BITS=64.
>
>
> --
> ldv

Re: [PATCH] stat.2: fixed inode printing in example program

[Konstantin Bukin]

> Do you agree it solves this problem?

No, that does not solve the problem. There is still casting to a
signed resulting in printing negative inode.

Using %llu might be better since that would not require including an
extra header.

Mind I'll send a new patch?

On Sun, Sep 13, 2020 at 11:04 AM Alejandro Colomar
<colomar.6.4.3@gmail.com> wrote:
>
> Hi,
>
> Please, see the patch I just sent:
>
> https://lore.kernel.org/linux-man/20200913175506.576683-1-colomar.6.4.3@gmail.com/T/#u
>
> Do you agree it solves this problem?
>
> Cheers,
>
> Alex

Re: [PATCH] stat.2: fixed inode printing in example program

[Alejandro Colomar]

Hi Konstantin,

On 9/13/20 8:16 PM, Konstantin Bukin wrote:
>> Do you agree it solves this problem?
> 
> No, that does not solve the problem. There is still casting to a
> signed resulting in printing negative inode.

True.  The definition of ino_t is unsigned, so the example was wrong
from the beginning.

> 
> Using %llu might be better since that would not require including an
> extra header.

We just had that discussion in this thread:

https://lore.kernel.org/linux-man/20200911231411.28406-1-colomar.6.4.3@gmail.com/T/#m971e4dcfae5f25e0f26c906679aa7176b6786bdf

We decided to go for [u]intmax_t.  But the patch has not yet been
applied, so you arrive just in time to give reasons against it.

> 
> Mind I'll send a new patch?
Sure.

Cheers,

Alex

Re: [PATCH] stat.2: fixed inode printing in example program

[Konstantin Bukin]

> > Mind I'll send a new patch?
> Sure.

The new patch uses "%lld" instead of "%ld".

Thank you,
Konsstantin.


On Sun, Sep 13, 2020 at 11:30 AM Alejandro Colomar
<colomar.6.4.3@gmail.com> wrote:
>
> Hi Konstantin,
>
> On 9/13/20 8:16 PM, Konstantin Bukin wrote:
> >> Do you agree it solves this problem?
> >
> > No, that does not solve the problem. There is still casting to a
> > signed resulting in printing negative inode.
>
> True.  The definition of ino_t is unsigned, so the example was wrong
> from the beginning.
>
> >
> > Using %llu might be better since that would not require including an
> > extra header.
>
> We just had that discussion in this thread:
>
> https://lore.kernel.org/linux-man/20200911231411.28406-1-colomar.6.4.3@gmail.com/T/#m971e4dcfae5f25e0f26c906679aa7176b6786bdf
>
> We decided to go for [u]intmax_t.  But the patch has not yet been
> applied, so you arrive just in time to give reasons against it.
>
> >
> > Mind I'll send a new patch?
> Sure.
>
> Cheers,
>
> Alex

Re: [PATCH] stat.2: fixed inode printing in example program

[Alejandro Colomar]

On 9/13/20 8:42 PM, Konstantin Bukin wrote:
>>> Mind I'll send a new patch?
>> Sure.
> 
> The new patch uses "%lld" instead of "%ld".
> 
> Thank you,
> Konsstantin.

BTW, please reply to the thread when submitting a new version of a
patch, so the thread can be easily followed.


If you use git send-email, this might help:

https://burzalodowa.wordpress.com/2013/10/05/how-to-send-patches-with-git-send-email/

Thanks,

Alex

[PATCH] stat.2: fixed inode printing in example program

[Konstantin Bukin]

inode numbers are expected to be positive. Casting them to a signed type
may result in printing negative values. E.g. running example program on
the following file:

$ ls -li test.txt
9280843260537405888 -r--r--r-- 1 kbukin hardware 300 Jul 21 06:36 test.txt

resutls in the following output:

$ ./example test.txt
ID of containing device:  [0,480]
File type:                regular file
I-node number:            -9165900813172145728
Mode:                     100444 (octal)
Link count:               1
Ownership:                UID=2743   GID=30
Preferred I/O block size: 32768 bytes
File size:                300 bytes
Blocks allocated:         8
Last status change:       Tue Jul 21 06:36:50 2020
Last file access:         Sat Sep 12 14:13:38 2020
Last file modification:   Tue Jul 21 06:36:50 2020

Such erroneous reporting happens for inode values greater than maximum
value which can be stored in signed long. Casting does not seem to be
necessary here. Printing inode as unsigned long long fixes the issue.
---
 man2/stat.2 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/man2/stat.2 b/man2/stat.2
index 7e5417480..82eaefcda 100644
--- a/man2/stat.2
+++ b/man2/stat.2
@@ -681,7 +681,7 @@ main(int argc, char *argv[])
     default:       printf("unknown?\en");                break;
     }
 
-    printf("I\-node number:            %ld\en", (long) sb.st_ino);
+    printf("I\-node number:            %llu\en", (unsigned long long) sb.st_ino);
 
     printf("Mode:                     %lo (octal)\en",
             (unsigned long) sb.st_mode);
-- 
2.17.0

Re: [PATCH] stat.2: fixed inode printing in example program

[Konstantin Bukin]

I mentioned cast in the commit message. Sending a new patch with a
revised commit message.

On Sun, Sep 13, 2020 at 12:12 PM Konstantin Bukin <kbukin@gmail.com> wrote:
>
> inode numbers are expected to be positive. Casting them to a signed type
> may result in printing negative values. E.g. running example program on
> the following file:
>
> $ ls -li test.txt
> 9280843260537405888 -r--r--r-- 1 kbukin hardware 300 Jul 21 06:36 test.txt
>
> resutls in the following output:
>
> $ ./example test.txt
> ID of containing device:  [0,480]
> File type:                regular file
> I-node number:            -9165900813172145728
> Mode:                     100444 (octal)
> Link count:               1
> Ownership:                UID=2743   GID=30
> Preferred I/O block size: 32768 bytes
> File size:                300 bytes
> Blocks allocated:         8
> Last status change:       Tue Jul 21 06:36:50 2020
> Last file access:         Sat Sep 12 14:13:38 2020
> Last file modification:   Tue Jul 21 06:36:50 2020
>
> Such erroneous reporting happens for inode values greater than maximum
> value which can be stored in signed long. Casting does not seem to be
> necessary here. Printing inode as unsigned long long fixes the issue.
> ---
>  man2/stat.2 | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/man2/stat.2 b/man2/stat.2
> index 7e5417480..82eaefcda 100644
> --- a/man2/stat.2
> +++ b/man2/stat.2
> @@ -681,7 +681,7 @@ main(int argc, char *argv[])
>      default:       printf("unknown?\en");                break;
>      }
>
> -    printf("I\-node number:            %ld\en", (long) sb.st_ino);
> +    printf("I\-node number:            %llu\en", (unsigned long long) sb.st_ino);
>
>      printf("Mode:                     %lo (octal)\en",
>              (unsigned long) sb.st_mode);
> --
> 2.17.0
>

[PATCH] stat.2: fixed inode printing in example program

[Konstantin Bukin]

inode numbers are expected to be positive. Casting them to a signed type
may result in printing negative values. E.g. running example program on
the following file:

$ ls -li test.txt
9280843260537405888 -r--r--r-- 1 kbukin hardware 300 Jul 21 06:36 test.txt

resutls in the following output:

$ ./example test.txt
ID of containing device:  [0,480]
File type:                regular file
I-node number:            -9165900813172145728
Mode:                     100444 (octal)
Link count:               1
Ownership:                UID=2743   GID=30
Preferred I/O block size: 32768 bytes
File size:                300 bytes
Blocks allocated:         8
Last status change:       Tue Jul 21 06:36:50 2020
Last file access:         Sat Sep 12 14:13:38 2020
Last file modification:   Tue Jul 21 06:36:50 2020

Such erroneous reporting happens for inode values greater than maximum
value which can be stored in signed long. Printing inode as unsigned
long long fixes the issue.
---
 man2/stat.2 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/man2/stat.2 b/man2/stat.2
index 7e5417480..82eaefcda 100644
--- a/man2/stat.2
+++ b/man2/stat.2
@@ -681,7 +681,7 @@ main(int argc, char *argv[])
     default:       printf("unknown?\en");                break;
     }
 
-    printf("I\-node number:            %ld\en", (long) sb.st_ino);
+    printf("I\-node number:            %llu\en", (unsigned long long) sb.st_ino);
 
     printf("Mode:                     %lo (octal)\en",
             (unsigned long) sb.st_mode);
-- 
2.17.0

Re: [PATCH] stat.2: fixed inode printing in example program

[Michael Kerrisk (man-pages)]

Hello Konstantin,

On 9/13/20 9:40 PM, Konstantin Bukin wrote:
> inode numbers are expected to be positive. Casting them to a signed type
> may result in printing negative values. E.g. running example program on
> the following file:
> 
> $ ls -li test.txt
> 9280843260537405888 -r--r--r-- 1 kbukin hardware 300 Jul 21 06:36 test.txt
> 
> resutls in the following output:
> 
> $ ./example test.txt
> ID of containing device:  [0,480]
> File type:                regular file
> I-node number:            -9165900813172145728
> Mode:                     100444 (octal)
> Link count:               1
> Ownership:                UID=2743   GID=30
> Preferred I/O block size: 32768 bytes
> File size:                300 bytes
> Blocks allocated:         8
> Last status change:       Tue Jul 21 06:36:50 2020
> Last file access:         Sat Sep 12 14:13:38 2020
> Last file modification:   Tue Jul 21 06:36:50 2020
> 
> Such erroneous reporting happens for inode values greater than maximum
> value which can be stored in signed long. Printing inode as unsigned
> long long fixes the issue.
> ---
>  man2/stat.2 | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/man2/stat.2 b/man2/stat.2
> index 7e5417480..82eaefcda 100644
> --- a/man2/stat.2
> +++ b/man2/stat.2
> @@ -681,7 +681,7 @@ main(int argc, char *argv[])
>      default:       printf("unknown?\en");                break;
>      }
>  
> -    printf("I\-node number:            %ld\en", (long) sb.st_ino);
> +    printf("I\-node number:            %llu\en", (unsigned long long) sb.st_ino);
>  
>      printf("Mode:                     %lo (octal)\en",
>              (unsigned long) sb.st_mode);

Thanks for your patches and problem report. But, it seems best to 
go with Alejandro's suggestion to use uintmax_t + %ju. See the
patch that Alejandro has sent.

Thanks,

Michael


-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/

Re: [PATCH] stat.2: fixed inode printing in example program

[Konstantin Bukin]

Fair enough, cast is needed. I replied in the main thread with a fixed patch.

Thank you,
Konstantin.

On Sun, Sep 13, 2020 at 11:43 AM Alejandro Colomar
<colomar.6.4.3@gmail.com> wrote:
>
> > Such erroneous reporting happens for inode values greater than maximum
> > value which can be stored in signed long. Casting does not seem to be
> > necessary here. Printing inode as unsigned long long fixes the issue.
>
> The cast is necessary.
>
> Relevant standard: C18 §6.5.2.2 6
>
> Details: 'ino_t' may (and will likely) be defined as 'unsigned long'.
> If you try to print an 'unsigned long' with "%llu" you will:
> - have Undefined Behavior (and a warning) if 'sizeof(long) < sizeof(long
> long)'.
> - have a warning if 'sizeof(long) == sizeof(long long)'.
>
> So if you want to use 'unsigned long long', you'll need to cast.
>
> Cheers,
>
> Alex

Re: [PATCH] stat.2: fixed inode printing in example program

[Alejandro Colomar]

> Such erroneous reporting happens for inode values greater than maximum
> value which can be stored in signed long. Casting does not seem to be
> necessary here. Printing inode as unsigned long long fixes the issue.

The cast is necessary.

Relevant standard: C18 §6.5.2.2 6

Details: 'ino_t' may (and will likely) be defined as 'unsigned long'.
If you try to print an 'unsigned long' with "%llu" you will:
- have Undefined Behavior (and a warning) if 'sizeof(long) < sizeof(long
long)'.
- have a warning if 'sizeof(long) == sizeof(long long)'.

So if you want to use 'unsigned long long', you'll need to cast.

Cheers,

Alex

[PATCH] stat.2: fixed inode printing in example program

[Konstantin Bukin]

inode numbers are expected to be positive. Casting them to a signed type
may result in printing negative values. E.g. running example program on
the following file:

$ ls -li test.txt
9280843260537405888 -r--r--r-- 1 kbukin hardware 300 Jul 21 06:36 test.txt

resutls in the following output:

$ ./example test.txt
ID of containing device:  [0,480]
File type:                regular file
I-node number:            -9165900813172145728
Mode:                     100444 (octal)
Link count:               1
Ownership:                UID=2743   GID=30
Preferred I/O block size: 32768 bytes
File size:                300 bytes
Blocks allocated:         8
Last status change:       Tue Jul 21 06:36:50 2020
Last file access:         Sat Sep 12 14:13:38 2020
Last file modification:   Tue Jul 21 06:36:50 2020

Such erroneous reporting happens for inode values greater than maximum
value which can be stored in signed long. Casting does not seem to be
necessary here. Printing inode as unsigned long long fixes the issue.
---
 man2/stat.2 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/man2/stat.2 b/man2/stat.2
index 7e5417480..89990e581 100644
--- a/man2/stat.2
+++ b/man2/stat.2
@@ -681,7 +681,7 @@ main(int argc, char *argv[])
     default:       printf("unknown?\en");                break;
     }
 
-    printf("I\-node number:            %ld\en", (long) sb.st_ino);
+    printf("I\-node number:            %llu\en", sb.st_ino);
 
     printf("Mode:                     %lo (octal)\en",
             (unsigned long) sb.st_mode);
-- 
2.17.0