last day (14 days later) » 

12:23 PM
0
Q: buffer overflow during memory reallocation

kartikeykant18I was making a C program to convert 12 hour clock into 24 hour clock with the format of input being HH:MM:SSAM or HH:MM:SSPM and 24hr clock output being HH:MM:SS #include <stdio.h> #include <stdlib.h> int main(){ char *time = malloc(11 * sizeof(char)); scanf("%11s", time); if (time[...

 
why would you call free(time) after realloc?
 
Allocate +1 for NUL terminator.
 
alk
If really using a C compiler, just remove all those useless cast, they aren't needed nor recommended in any way.
sizeof(char) equals 1 by definition.
time is a commonly used identifier, better use your own.
 
@alk by cast you mean atoi() function?
 
alk
No, I mean the char* like in (char*)malloc(...
atoi() is unreliable as well, as it does not provide error checking, better use strtol().
 
DYZ
12:23 PM
If the original size of the variable is 10 bytes, why bother shrinking it to 8 bytes? Do 2 bytes make any significant difference?
 
alk
To define a C-"string" with N characters allocate N+1. Mind the 0-terminator.
 
in 24hr clock output I need HH:MM:SS so i shrink it to 8 bytes
 
as @BLUEPIXY says, your inital malloc isn't big enough. You need 11 bytes to hold HH:MM:SSAM plus the NULL terminator.
 
alk
Taking unlimited user input like here scanf("%s",... introduces the risk of overflowing the input buffer, better do scanf("%9s", .... Again: Mind the 0-terminator.
 
you do not need to shrink the buffer to the exact size. The end of a C string is defined by the null ('\0') not by length. THis is your error, you do not allow space for the null, your strpintf to str has the same problem, you need 3 chars not 2
 
12:23 PM
you could refactor this by pulling the identical realloc() calls out (above) the inner if/else, and the moving the identical printf() outside (below), and then you have an empty if() {} block which could then be removed.
 
alk
Here atoi(str); you pass the address of the 1st element to 2-element-sized char-array. atoi() expects a C-"string", which str isn't. Again^2 : Mind the 0-terminator.
 
@kartikeykant18 HH:MM:SS+ NUL = 8 +1. HH:MM:SSAM+NUL = 10+1, char str[2]; --> char str[2+1];, char milstr[2]; --> char milstr[2+1];
 
alk
Here sprintf(milstr, "%d", milhours); you (most likely) print 2 bytes into a 2-element-sized char-array, where you need 3 bytes. The third is for ... you name it: The 0- terminator.
Editing your question in place is not a good idea after comments and/or questions have been given as such edits may render the answers/comment und-understandable. Better add edits as updates/additions to your question.
 
scanf("%11s", time); --> scanf("%10s", time); and You forgot one as time = realloc(time, (8 * sizeof(char)));.
 
@BLUEPIXY null terminator is not something inputted by the user, is that why?
when i shrink the size of time using realloc does it remove the last 2 elements inputted by the user i.e. AM or PM?
 
12:23 PM
like scanf("%10s", time); and sprintf(str, "%c%c", time[0], time[1]); add it at the end.
Although it is not necessarily deleted, it can not be used.
 
but in output it prints out PM or AM, rest is correct
 
So, It is necessary to modify the program. E.g time[8] = 0; printf("%s\n", time);
If realloc changes memory size successfully, the new memory block may be another location or may be the original address. In any case do not access the part exceeding the specified size. Do not expect that the shrunk part will be initialized with 0. Probably not like that.
 
So, seems no one told you that your code has a classic bug. i.e. x = realloc(x, ...);
 

last day (14 days later) »