I have to take numbers represented as strings and perform an addition operation on them. This is for homework, and what I have done so far sort of works. It outputs the correct output, but my program also needs to be free of memory errors and it certainly is not that.
Any help someone could provide on whats going on with these uninitialized variables would be helpful. I thought it was the use of malloc instead of calloc, but I changed that as well, and still no dice.
Here is my output:
./a.out
strlen: 12
Old: 12122334455
New: 012122334455
Result: 167244668910
Here is my code: /* * File: strmath.c */
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <limits.h>
#include <ctype.h>
int cmp(char *, char *, int);
void add(char *, char *, int);
char * reverse(char *);
void strReverse(char *);
char *strpadleft(char * , char, int );
int main()
{
char str1[] = "155122334455",
str2[] = "12122334455"; //malloc(10);
int len = strlen(str1) < strlen(str2)?strlen(str2):strlen(str1);
printf("strlen: %d\n", len);
// int result = cmp(str1, str2, len);
// if(result > 0)
// printf("%s > %s\n", str1, str2);
// else if(result < 0)
// printf("%s > %s\n", str2, str1);
// else
// printf("%s == %s\n", str1, str2);
add(str1, str2, len);
return 0;
}
int cmp(char *str1, char *str2, int longest) {
if (strlen(str1) < strlen(str2))
return -1;
else if (strlen(str1) > strlen(str2))
return 1;
else
return strncmp(str1, str2, longest);
}
void add(char *str1, char *str2, int longest) {
char * num1 = strpadleft(str1, '0', longest);
char * num2 = strpadleft(str2, '0', longest);
strReverse(num1);
strReverse(num2);
char * result = calloc(longest + 2 , sizeof(char));
int i, x, carry=0;
for (i = 0; i < longest;i++) {
x = (num1[i] - '0') + (num2[i] - '0') + carry;
if (x >= 10) {
carry = 1;
x -= 10;
}
else {
carry = 0;
}
result[i] = x + '0';
}
if (carry == 1) { result[i+1] = '1';}
strReverse(result);
printf("Result: %s\n", result );
//free(result);
//free(num1);
//free(num2);
}
char *strpadleft(char * string, char pad, int padSize)
{
int ssize = strlen(string);
if (padSize <= ssize) return string;
char *padded = calloc((padSize + 1) , sizeof(char));
memset(padded, pad, padSize-ssize);
padded = strcat(padded, string);
printf("Old: %s\nNew: %s\n", string, padded);
return padded;
}
void strReverse(char* str){
int length = strlen(str);
char temp;
int i,j;
for(i = 0, j = length-1;i < (length-1)/2; i++, j--){
temp = str[i];
str[i]=str[j];
str[j] = temp;
}
return;
}
And here is my valgrind output (the full output of valgrind is at http://textsnip.com/ae3c9a since it was too much for SO):
==17115== Memcheck, a memory error detector
==17115== Copyright (C) 2002-2011, and GNU GPL'd, by Julian Seward et al.
==17115== Using Valgrind-3.7.0 and LibVEX; rerun with -h for copyright info
==17115== Command: a.out
==17115==
==17115== Conditional jump or move depends on uninitialised value(s)
==17115== at 0x414F76: __linkin_atfork (in /p2/hj/jsiegal/csc352/assg5/a.out)
==17115== by 0x404383: ptmalloc_init.part.8 (in /p2/hj/jsiegal/csc352/assg5/a.out)
==17115== by 0x407E64: malloc_hook_ini (in /p2/hj/jsiegal/csc352/assg5/a.out)
==17115== by 0x4538E2: _dl_init_paths (in /p2/hj/jsiegal/csc352/assg5/a.out)
==17115== by 0x415788: _dl_non_dynamic_init (in /p2/hj/jsiegal/csc352/assg5/a.out)
==17115== by 0x416002: __libc_init_first (in /p2/hj/jsiegal/csc352/assg5/a.out)
==17115== by 0x40173C: (below main) (in /p2/hj/jsiegal/csc352/assg5/a.out)
==17115== Uninitialised value was created
==17115== at 0x45018A: brk (in /p2/hj/jsiegal/csc352/assg5/a.out)
==17115== by 0x41370B: sbrk (in /p2/hj/jsiegal/csc352/assg5/a.out)
==17115== by 0x401C95: __pthread_initialize_minimal (in /p2/hj/jsiegal/csc352/assg5/a.out)
==17115== by 0x4016F0: (below main) (in /p2/hj/jsiegal/csc352/assg5/a.out)
...
==17115== Use of uninitialised value of size 8
==17115== at 0x405B27: _int_malloc (in /p2/hj/jsiegal/csc352/assg5/a.out)
==17115== by 0x408E29: calloc (in /p2/hj/jsiegal/csc352/assg5/a.out)
==17115== by 0x401418: add (strmath.c:51)
==17115== by 0x4012AB: main (strmath.c:31)
==17115== Uninitialised value was created by a stack allocation
==17115== at 0x453ADD: _dl_init_paths (in /p2/hj/jsiegal/csc352/assg5/a.out)
Fixed:
/*
* File: strmath.c
*/
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <limits.h>
#include <ctype.h>
int cmp(char *, char *, int);
void add(char *, char *, int);
char * reverse(char *);
void strReverse(char *);
void strpadleft(char * , char *, char, int );
int main()
{
char str1[] = "155122334455",
str2[] = "12122334455"; //malloc(10);
int len = strlen(str1) < strlen(str2)?strlen(str2):strlen(str1);
printf("strlen: %d\n", len);
// int result = cmp(str1, str2, len);
// if(result > 0)
// printf("%s > %s\n", str1, str2);
// else if(result < 0)
// printf("%s > %s\n", str2, str1);
// else
// printf("%s == %s\n", str1, str2);
add(str1, str2, len);
return 0;
}
int cmp(char *str1, char *str2, int longest) {
if (strlen(str1) < strlen(str2))
return -1;
else if (strlen(str1) > strlen(str2))
return 1;
else
return strncmp(str1, str2, longest);
}
void add(char *str1, char *str2, int longest) {
char * num1 = malloc((longest + 1) * sizeof(char));
memset(num1, '\0', longest+1);
strpadleft(str1, num1, '0', longest);
char * num2 = malloc((longest + 1) * sizeof(char));
memset(num2, '\0', longest+1);
strpadleft(str2, num2, '0', longest);
strReverse(num1);
strReverse(num2);
char * result = malloc(longest + 2 * sizeof(char));
memset(result, '\0', longest+2);
int i, x, carry=0;
for (i = 0; i < longest;i++) {
x = (num1[i] - '0') + (num2[i] - '0') + carry;
if (x >= 10) {
carry = 1;
x -= 10;
}
else {
carry = 0;
}
result[i] = x + '0';
}
if (carry == 1) { result[i+1] = '1';}
strReverse(result);
printf("Result: %s\n", result );
free(result);
free(num1);
free(num2);
}
void strpadleft(char * string, char *padded, char pad, int padSize)
{
int ssize = strlen(string);
if (padSize <= ssize) {
strcpy(padded, string);
return;
}
//char *padded = malloc((padSize + 1) * sizeof(char));
//memset(padded, '\0', padSize);
memset(padded, pad, padSize-ssize);
padded = strcat(padded, string);
printf("Old: %s\nNew: %s\n", string, padded);
return;
}
void strReverse(char* str){
int length = strlen(str);
char temp;
int i,j;
for(i = 0, j = length-1;i < (length-1)/2; i++, j--){
temp = str[i];
str[i]=str[j];
str[j] = temp;
}
return;
}
And the valgrind output:
==12778== Memcheck, a memory error detector
==12778== Copyright (C) 2002-2011, and GNU GPL'd, by Julian Seward et al.
==12778== Using Valgrind-3.7.0 and LibVEX; rerun with -h for copyright info
==12778== Command: a.out
==12778==
strlen: 12
Old: 12122334455
New: 012122334455
Result: 167244668910
==12778==
==12778== HEAP SUMMARY:
==12778== in use at exit: 0 bytes in 0 blocks
==12778== total heap usage: 3 allocs, 3 frees, 40 bytes allocated
==12778==
==12778== All heap blocks were freed -- no leaks are possible
==12778==
==12778== For counts of detected and suppressed errors, rerun with: -v
==12778== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 2 from 2)
The valgrind log does not represent your program execution. It actually got crazy with libc internal functions. The apparent reason for this is that a proper suppression file (a list of false positive memory access/allocation errors) for the exact libc version you're using is missing. Valgrind should have worked it out automatically, but this didn't happen. You need to either recompile valgrind in your system so that it detects the right libc again, or it's completely unable to auto-detect it (and then you need to manually build a suppresion file). See, for example:
And also, the suppression file documentation:
Now, for the code, it's clear that each calloc does not have a corresponding free. I'll copy paste my valgrind execution for you to understand what's the real valgrind output:
(...)
==26074== 13 bytes in 1 blocks are definitely lost in loss record 1 of 2
==26074== at 0x4C29E46: calloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==26074== by 0x400A15: strpadleft (in /tmp/a)
==26074== by 0x40081A: add (in /tmp/a)
==26074== by 0x4007D4: main (in /tmp/a)
==26074==
==26074== 14 bytes in 1 blocks are definitely lost in loss record 2 of 2
==26074== at 0x4C29E46: calloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==26074== by 0x40084B: add (in /tmp/a)
==26074== by 0x4007D4: main (in /tmp/a)
This shows what's happening: no calloc has a matching free. In the add function, there's a calloc for result. A free is missing. In strpadleft, there's a calloc for the result function. So it returns a dinamically allocated variable, which should be freed by the caller. That's missing. The real problem, however is that strpadleft might return early (return string) without allocating anything. There's no way for the caller know if the function is allocating a new value or not. This is wrong. Either inform the caller what was the operation done or make allocation uniform (allocate always).
Conclusion: freeing memory is a simple matter of matching all allocations. You keep track of the allocations with your variables. When you don't need them anymore, free them.