> We found an issue with the "ls -l" command not showing the correct file size.

Can you show me the example of ls -l output you are observing?

Below is the output. Notice that all the sizes are the same even for soft links. It was not like this with an earlier version 1.16.x. Let me know if you need more information. Thanks.

# ls -l /etc
lrwxrwxrwx    1 root     root       1050012 Oct 26 17:26 bluetooth -> /system/etc/bluetooth
lrwxrwxrwx    1 root     root       1050012 Oct 26 17:26 dbus.conf -> /system/etc/dbus.conf
-rwxr-xr-x    1 root     root       1050012 Sep 14 11:28 fstab
lrwxrwxrwx    1 root     root       1050012 Oct 26 17:26 gps.conf -> /system/etc/gps.conf
drwxr-xr-x    2 root     root       1050012 Oct 26 17:25 init.d
-rwxr-xr-x    1 root     root       1050012 Sep 14 11:28 inittab
-rwxrwxr-x    1 root     root       1050012 Oct 26 17:25 media_profiles.xml
lrwxrwxrwx    1 root     root       1050012 Oct 26 17:25 mtab -> ../proc/mounts
drwxr-xr-x    3 root     root       1050012 Oct 26 17:25 rc.d
lrwxrwxrwx    1 root     root       1050012 Oct 26 17:40 sensors.conf
lrwxrwxrwx    1 root     root       1050012 Oct 26 17:26 vold.fstab -> /system/etc/vold.fstab

It doesn't happen with my busybox. Please attach your .config file

Created attachment 3722 [details]
config file that shows the issue

Hi,

Attached is the config file that we use. Thanks.

Can't reproduce even with your .config.

Please add the following three lines marked with ^^^^^^^^ below in ls.c in my_stat() function, recompile, rerun "ls -l /etc", post the resulting output.



        if ((option_mask32 & OPT_L) || force_follow) {
#if ENABLE_SELINUX
                if (is_selinux_enabled())  {
                         getfilecon(fullname, &cur->sid);
                }
#endif
bb_error_msg("stat '%s'", fullname);
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
                if (stat(fullname, &statbuf)) {
                        bb_simple_perror_msg(fullname);
                        G.exit_code = EXIT_FAILURE;
                        free(cur);
                        return NULL;
                }
                cur->dn_mode_stat = statbuf.st_mode;
        } else {
#if ENABLE_SELINUX
                if (is_selinux_enabled()) {
                        lgetfilecon(fullname, &cur->sid);
                }
#endif
bb_error_msg("lstat '%s'", fullname);
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
                if (lstat(fullname, &statbuf)) {
                        bb_simple_perror_msg(fullname);
                        G.exit_code = EXIT_FAILURE;
                        free(cur);
                        return NULL;
                }
                cur->dn_mode_lstat = statbuf.st_mode;
        }

        /* cur->dstat = statbuf: */
        cur->dn_mode   = statbuf.st_mode  ;
        cur->dn_size   = statbuf.st_size  ;
bb_error_msg("size:%llu", (long long)(cur->dn_size));
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

actually we fixed the issue by doing a similar change as the code: bb_error_msg("size:%llu", (long long)(cur->dn_size));

The casting worked and displayed the file sizes correctly. Just wanted to know if you have any known issues or fixes to what we are seeing. I will with the above changes and post the output from the resulting changes.

(In reply to comment #6)
> actually we fixed the issue by doing a similar change as the code:
> bb_error_msg("size:%llu", (long long)(cur->dn_size));
> 
> The casting worked and displayed the file sizes correctly.

Can you elaborate?

Or better yet, paste the output of "ls -l /etc" output verbatim?

Created attachment 3734 [details]
Attached is the output from the debug statements that was asked to run

What I meant about the fix was I have changed the line,
column += printf("%9"OFF_FMT"u ", dn->dn_size);
to 
column += printf("%9"OFF_FMT"u ", (unsigned long long) dn->dn_size);

and I have got the "ls -l" to display the correct sizes. However attached is the output from the three lines that you asked me to add and test. Note that I have not put in our fix that I mentioned above with this test output. It is unchanged busybox code with your debug code. Thanks.

(In reply to comment #8)
> Created attachment 3734 [details]
> Attached is the output from the debug statements that was asked to run
> 
> What I meant about the fix was I have changed the line,
> column += printf("%9"OFF_FMT"u ", dn->dn_size);
> to 
> column += printf("%9"OFF_FMT"u ", (unsigned long long) dn->dn_size);

If that change fixes the problem for you, it means that off_t (dn->dn_size has this type) and OFF_FMT don't match. This means that not only ls will be affected.

The problem seems to be that OFF_FMT string is "ll" but off_t is not "long long".

Before we start digging deeper, let's confirm this. Add these lines at the beginning of ls_main():

printf("sizeof(off_t):%d\n"    , (int) sizeof(off_t));
printf("sizeof(long long):%d\n", (int) sizeof(long long));
printf("sizeof(dn_size):%d\n"  , (int) sizeof( ((struct dnode*)NULL)->dn_size ));
printf("OFF_FMT:'"OFF_FMT"'\n");

recompile, run ls applet, and post the output of these four lines.

Here is the output:
sizeof(off_t):4
sizeof(long long):8
sizeof(dn_size):4
OFF_FMT:'ll'

Just to add more information to this issue android's version of bionic libc's stat structure uses long long for file size to store the file size. Cyanogen's android project has added the below change to their integrated version of libb.h.

#ifdef __BIONIC__
/* bionic uses stat64 which has long long file sizes, whereas off_t is only long bits */
typedef long long filesize_t;
#define FILESIZE_FMT "ll"
#else
typedef off_t filesize_t;
#define FILESIZE_FMT OFF_FMT
#endif

I added the check for this condition:

/* Users report bionic to use 32-bit off_t even if LARGEFILE support is requested.
 * We misdetected that. Don't let it build:
 */
struct BUG_off_t_size_is_misdetected {
        char BUG_off_t_size_is_misdetected[sizeof(off_t) == sizeof(uoff_t) ? 1 : -1];
};

Bionic users are probably better switching CONFIG_LFS off.

I consider this bug closed for now.

If you know a generic method to detect 32-bit off_t, please let me know.