C++

Forum

 
strcat and char initialized with ""

Pages: 12
philonto (36)
Hi,

I have a function simplified like this:
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
 
int main() {
    char skipped[1024] = "";
    static char* partitions[] = { "/part0",
                    "/part1",
                    "/part2",
                    "/part3",
                    "/part4",
                    "/part5",
                    "/part6",
                    "/part7",
                    "/part8",
                    "/part9",
                    NULL
    };

    int i;
    for(i=0; partitions[i] != NULL; i++) {
        strcat(skipped, " - ");
        strcat(skipped, partitions[i]);
    }
    printf("%s\n", skipped);
    return 0;
}


This code runs perfect in gcc
When I compile in android tree, on my device it runs fine
However, for some people, on a different device, the printf will not output what expected, but some garbage, like if char skipped[] was not properly initialized

Is it mandatory to initialze char skipped[1024] = {0} for strcat?
Why my initializer "" works for some and not for others?

I am sure it doesn't overload as I control the input in the same function
vlad from moscow (6539)
There is no difference between the two initializations. In both cases all elements of the array will be set to 0. You could even do not use an explicit initialization because the array is static and will be initialized by the compiler.
But in any case the second array shall have type

static const char* partitions[]

EDIT: I am sorry. The first array is not static. So it shall be initialized by using any of the two methods.
Last edited on
philonto (36)
By setting it to "", only first element will be set to 0, not whole array nulled. But, also, in my thinking, it shouldn't matter
Well, here's the whole function, if some one have an idea why sometimes my print of char skipped[] returns garbadge

Do not be afraid of the long code, just look at the char skipped[1024] = ""; through code till ui_print(">> Unknown partitions size (%d):%s\n", ret, skipped);

ui_print is just a printf() defined function


1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
 
static int check_backup_size() {
    int total_mb = (int)(Total_Size / 1048576LLU);
    int used_mb = (int)(Used_Size / 1048576LLU);
    int free_mb = (int)(Free_Size / 1048576LLU);
    int free_percent = free_mb * 100 / total_mb;
    Before_Used_Size = Used_Size; // save Used_Size to refresh data written stats later
    Backup_Size = 0;

    static char* partitions[] = { "/recovery",
                    "/boot",
                    "/wimax",
                    "/modem",
                    "/radio",
                    "/efs",
                    "/misc",
                    "/system",
                    "/preload",
                    "/data",
                    "/datadata",
                    "/cache",
                    "/sd-ext",
                    NULL
    };

    int preload_status = 0;
    if ((is_custom_backup && backup_preload) || (!is_custom_backup && nandroid_add_preload))
        preload_status = 1;

    int backup_status[] = {backup_recovery,
            backup_boot,
            backup_wimax,
            backup_modem,
            backup_modem,
            backup_efs,
            backup_misc,
            backup_system,
            preload_status,
            backup_data,
            backup_data,
            backup_cache,
            backup_sdext,
    };

    char skipped[1024] = "";
    int ret = 0;
    struct statfs s;
    Volume* vol;

    int i;
    for(i=0; partitions[i] != NULL; i++) {
        if (!backup_status[i])
            continue;
        if (strcmp(partitions[i], "/data") == 0 && is_data_media())
            continue;
        if (strcmp(partitions[i], "/datadata") == 0 && !has_datadata())
            continue;
        
        vol = volume_for_path(partitions[i]);
        if (vol == NULL || 0 != stat(vol->device, &s)) continue;

        if (Is_Image(partitions[i]) == 0) {
            if (Find_Partition_Size(partitions[i]) == 0) {
                Backup_Size += Total_Size;
                LOGI("%s backup size (/proc)=%lluMb\n", partitions[i], Total_Size / 1048576LLU); // debug
            } else {
                ret++;
                strcat(skipped, " - ");
                strcat(skipped, partitions[i]);
            }
        } else if (Is_File_System(partitions[i]) == 0) {
            if (0 == ensure_path_mounted(vol->mount_point) && 0 == Get_Size_Via_statfs(vol->mount_point)) {
                Backup_Size += Used_Size;
                LOGI("%s backup size (stat)=%lluMb\n", partitions[i], Used_Size / 1048576LLU); // debug
            } else {
                ret++;
                strcat(skipped, " - ");
                strcat(skipped, partitions[i]);
            }
        } else {
            ret++;
            strcat(skipped, " - Unknown file system: ");
            strcat(skipped, partitions[i]);
        }
    }

    if (backup_data && is_data_media()) {
        if (0 == ensure_path_mounted("/data") && 0 == Get_Size_Via_statfs("/data")) {
            unsigned long long data_backup_size;
            data_backup_size = Used_Size - Get_Folder_Size("/data/media");
            Backup_Size += data_backup_size;
            LOGI("/data backup size=%lluMb\n", data_backup_size / 1048576LLU); // debug
        } else {
            ret++;
            strcat(skipped, " - /data");
        }
    }

    char tmp[PATH_MAX];
    get_android_secure_path(tmp);
    if (backup_data && android_secure_ext) {
        unsigned long long andsec_size;
        andsec_size = Get_Folder_Size(tmp);
        Backup_Size += andsec_size;
        LOGI("%s backup size=%lluMb\n", tmp, andsec_size / 1048576LLU); // debug
    }

    int backup_size_mb = (int)(Backup_Size / 1048576LLU);
    backup_size_mb += 50; // extra 50 Mb for security measures

    ui_print("\n>> Free space: %dMb (%d%%)\n", free_mb, free_percent);
    ui_print(">> Needed space: %dMb\n", backup_size_mb);
    if (ret)
        ui_print(">> Unknown partitions size (%d):%s\n", ret, skipped);

    if (free_percent < 3 || (default_backup_handler != tar_compress_wrapper && free_mb < backup_size_mb)) {
        if (!confirm_selection("Low free space! Continue anyway?", "Yes - Continue Nandroid Job"))
            return -1;
    }

    return 0;
}
vlad from moscow (6539)
@philonto

By setting it to "", only first element will be set to 0, not whole array nulled. But, also, in my thinking, it shouldn't matter


You are wrong. All elements of the array will be set to zero.
vlad from moscow (6539)
As I said above you shall declare partitions as

static const char * []
philonto (36)
Thank you again
Just for my understanding:
What is happening that can explain my issue and could be caused by partitions not being const argument for strcat?

Undefined behavior?
Last edited on
vlad from moscow (6539)
Maybe somewhere in the code you are trying to change some string literal.
Last edited on
philonto (36)
I followed the 3 functions where partitions[i] is sent as an argument.
They all receive it as function(const char* path) and none sends it away or modifies it (no dirname, nothing similar)

Really wired
Also, I read a bit more the manuals, and strcat could still receive a non const without any warning, as soon as we do not modify it

I understand it should be const, but I am confused why it ends like this
Last edited on
vlad from moscow (6539)
You can simply include some test output of the character array in your code to determine the place where the character array is being broken.
philonto (36)
Thanks, I will do that
vlad from moscow (6539)
Mayby you overwrite the memory occupied by the array somewhere in the code..
Last edited on
philonto (36)
yes, that's a potential issue
I am going to trace it
I will also rename it and add static const: could help to fix it too I guess
But knowing the source of teh issue would be better
Last edited on
philonto (36)
Ok, I got some debugging from concerned users

It seems like my partitions[i] is never corrupted
However, the first strcat seems to be failing like if target wasn't initialized

This still happens after using static const char* partitions[] (but clearly, it is not the partitions[i] being changed as it is still un-corrupted before the first strcat).

The first strcat operation seems to be adding over a non empty char skipped[1024]

Initializing it with {0} did not fix any thing as expected

Is it possible that the first strcat needs a previous sprintf or strcpy on target string under some circumstances?

Sorry, I ask here as I cannot reproduce the issue on my compiled program, and only relying on users logs

If it is not, does it mean a memory leak or overload somewhere else?
Last edited on
philonto (36)
I edited my function this way to debug it:
The LOGI functions will act just like a printf for debugging

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
 
static int check_backup_size() {
    // these are the size stats for backup_path we previously refreshed by calling Get_Size_Via_statfs()
    int total_mb = (int)(Total_Size / 1048576LLU);
    int used_mb = (int)(Used_Size / 1048576LLU);
    int free_mb = (int)(Free_Size / 1048576LLU);
    int free_percent = free_mb * 100 / total_mb;
    Before_Used_Size = Used_Size; // save Used_Size to refresh data written stats later
    Backup_Size = 0;

    // backable partitions
    static const char* Partitions_List[] = {"/recovery",
                    "/boot",
                    "/wimax",
                    "/modem",
                    "/radio",
                    "/efs",
                    "/misc",
                    "/system",
                    "/preload",
                    "/data",
                    "/datadata",
                    "/cache",
                    "/sd-ext",
                    NULL
    };

    int preload_status = 0;
    if ((is_custom_backup && backup_preload) || (!is_custom_backup && nandroid_add_preload))
        preload_status = 1;

    int backup_status[] = {backup_recovery,
            backup_boot,
            backup_wimax,
            backup_modem,
            backup_modem,
            backup_efs,
            backup_misc,
            backup_system,
            preload_status,
            backup_data,
            backup_data,
            backup_cache,
            backup_sdext,
    };

    // calculate needed space for backup
    // assume recovery and wimax always use a raw backup mode (Is_Image() = 0)
    char skipped_parts[1024];
    sprintf(skipped_parts, "test:");
    int ret = 0;
    struct statfs s;
    Volume* vol;

    int i;
    for(i=0; Partitions_List[i] != NULL; i++) {
        if (!backup_status[i])
            continue;
        // size of /data will be calculated later for /data/media devices to substract sdcard size from it
        LOGI("1-%s\n", Partitions_List[i]); // debug
        if (strcmp(Partitions_List[i], "/data") == 0 && is_data_media()) {
            LOGI("2-%s\n", Partitions_List[i]); // debug
            continue;
        }
        // redondant but keep for compatibility:
        // has_datadata() does a volume_for_path() != NULL check
        LOGI("3-%s\n", Partitions_List[i]); // debug
        if (strcmp(Partitions_List[i], "/datadata") == 0 && !has_datadata()) {
            LOGI("4-%s\n", Partitions_List[i]); // debug
            continue;
        }
        LOGI("5-%s\n", Partitions_List[i]); // debug
        vol = volume_for_path(Partitions_List[i]);
        LOGI("6-%s\n", Partitions_List[i]); // debug
        if (vol == NULL || 0 != stat(vol->device, &s)) continue;

        LOGI("6.1-%s\n", Partitions_List[i]); // debug
        if (Is_Image(Partitions_List[i]) == 0) {
            LOGI("7-%s\n", Partitions_List[i]); // debug
            if (Find_Partition_Size(Partitions_List[i]) == 0) {
                Backup_Size += Total_Size;
                LOGI("%s backup size (/proc)=%lluMb\n", Partitions_List[i], Total_Size / 1048576LLU); // debug
            } else {
                ret++;
                LOGI("8.0-%s - %s\n", Partitions_List[i], skipped_parts); // debug
                strcat(skipped_parts, " - ");
                LOGI("8.1-%s\n", skipped_parts); // debug
                strcat(skipped_parts, Partitions_List[i]);
                LOGI("8.2-%s\n", skipped_parts); // debug
            }
        } else if (Is_File_System(Partitions_List[i]) == 0) {
            // Get_Size_Via_statfs() will ensure vol->mount_point != NULL
            if (0 == ensure_path_mounted(vol->mount_point) && 0 == Get_Size_Via_statfs(vol->mount_point)) {
                Backup_Size += Used_Size;
                LOGI("%s backup size (stat)=%lluMb\n", Partitions_List[i], Used_Size / 1048576LLU); // debug
            } else {
                ret++;
                LOGI("9-%s\n", Partitions_List[i]); // debug
                strcat(skipped_parts, " - ");
                strcat(skipped_parts, Partitions_List[i]);
            }
        } else {
            ret++;
            LOGI("10-%s\n", Partitions_List[i]); // debug
            strcat(skipped_parts, " - Unknown file system: ");
            strcat(skipped_parts, Partitions_List[i]);
        }
    }
...


Affected users have this log:
1
2
3
4
5
6
7
8
9
10
 
I:1-/recovery
I:3-/recovery
I:5-/recovery
I:6-/recovery
I:6.1-/recovery
I:7-/recovery
E:Failed to find partition size '/recovery'
I:8.0-/recovery - dÓ;9Š>
I:8.1-dÓ;9Š> - 
I:8.2-dÓ;9Š> - /recovery


So, despite now initializing char skipped_parts[1024] with sprintf(skipped_parts, "test:"), the skipped_parts seems corrupted before first strcat call (log output 8.0-/recovery...)

I am lost here. Must I look outside the function for a memory leak or overload? Is there other things in my function that could explain it?
vlad from moscow (6539)
It is the result of the badly written code. When a code is written in a bad style you may be sure that somewhere in the ocde there is an error.
I am sure that the error has nothing common with the initialization of the array and strcat.
The best solution is to throw out this stupid code and rewrite the program from scratch.
Last edited on
philonto (36)
What is really stupid to be more constructive?
philonto (36)
by the way, the whole program is not mine, but from cyanogenmod team. I modified it to add some functions, while following the existing C style and functions to keep compatibility

So, I am rather limited as some functions cannot be modified, else maintaining updates would be a full time job going through thousandths of code lines

By the way, that code works for about 30 different devices, only one is affected, so it could be a memory issue with that still too new device or the experimental ported sources from cm-team...
Last edited on
philonto (36)
Ok, I could limit the issue here:
 
 
if (vol == NULL || 0 != stat(vol->device, &s)) continue;


before that line, my char skipped_parts[1024] have the good contents
after stat, it gets corrupted

It is exactly before and after that line

Any help how can stat cause this in my code?
JLBorges (13770)
Shouldn't it be either
1
2
3
4
 
// struct statfs s;
struct stat s ;
// ...
stat( ...., &s) ;


or
1
2
3
4
 
struct statfs s;
// ...
//stat( ...., &s) ; 
statfs(... , &s) ;
philonto (36)
My bad
So silly and stupid typo costing me a lot of time

Thank you again
Pages: 12