So, I'm facing an issue that I don't really understand. Please be kind I'm trying to learn C by myself !
I have a function that is called secureInput() that takes a pointer to a string and a size so that, when the user has to type some input I can be sure that there is no buffer overflow. Now, the thing is that I want to modify the string without copying it so instead modifying it directly by it's memory address but it just crashes when the second character in user input is assigned. See comments to see where it crashes.
#include <stdio.h>
#include <stdlib.h>
#include <assert.h>
int secureInput(char **str, int size);
int main(int argc, const char * argv[]) {
char *mystring = NULL; // Declaring it a null so that I use malloc later
secureInput(&mystring, 10);
printf("%s\n", mystring);
}
int secureInput(char **str, int size)
{
*str = (char*)malloc(sizeof(char) *size); // Because **str is a null pointer, I use malloc to allocate memory.
if (*str == NULL)
return -1;
int c = 0;
unsigned int count = 0;
while((c = getchar()) != '\n' && count < size)
/* Here is where it crashes.
* But changing the bellow line as : *str[0][count ] = c;
* works as expected. Also, using a temporary pointer
* and later using it to replace *str, is also working
*/
*str[count ] = c;
*str[count] = '\0';
return 0;
}
CodePudding user response：
So first off, you can pass a string as a char*, no need for char**. That is usually used for an array of strings when passed as argument. Then, if you want to use a fixed size array, a buffer, that has a constant, pre-defined size, don't use malloc. Dynamic memory allocation is always inefficient and risky, so only use it if absolutely necessary.
#include <stdio.h>
#include <stdlib.h>
#include <assert.h>
#include <string.h>
#define BUFFER_SIZE 10
int secureInput(char *str, int size);
int main(int argc, const char * argv[]) {
char mystring[BUFFER_SIZE]; // Declaring it a null so that I use malloc later
memset(mystring, 0, BUFFER_SIZE);
secureInput(mystring, BUFFER_SIZE);
printf("%s\n", mystring);
}
int secureInput(char *str, int size) {
char c = 0;
unsigned int count = 0;
c = getchar();
while(c != '\n' && count < size - 1) {
str[count ] = c;
c = getchar();
}
str[count] = '\0';
return 0;
}
EDIT:
I can see that there is still some confusion regarding the pointer arithmetic. Here is some address printing and a small figure, I hope it helps:
#include <stdio.h>
#include <stdlib.h>
#include <assert.h>
int secureInput(char **str, int size);
int main(int argc, const char * argv[]) {
char *mystring = NULL; // Declaring it a null so that I use malloc later
secureInput(&mystring, 10);
}
int secureInput(char **str, int size) {
*str = (char*)malloc(sizeof(char) *size); // Because **str is a null pointer, I use malloc to allocate memory.
(*str)[0] = 'a';
(*str)[1] = 'b';
(*str)[2] = 'c';
(*str)[3] = 0;
printf("address of the pointer that points to a pointer that points the first char of the array : %p\n", &str);
printf("value of the pointer that points to a pointer that points to the first char of the array : %p\n", str);
printf("address of the pointer that points to the first char of the array : %p\n", &(*str));
printf("value of the pointer that points to the first char of the array : %p\n", *str);
printf("address of the first char of the array: %p\n", &(**str));
printf("address of the seconds char of the array: %p\n", &((*str)[1]));
printf("value of the first char of the array : %c\n", **str);
printf("value of the second char of the array : %c\n", *(*str 1));
printf("value of the second char of the array : %c\n", (*str)[1]);
printf("*str[1] is the same as *(str[1]), which runs to a segmentation fault\n");
return 0;
}
The output:
address of the pointer that points to a pointer that points the first char of the array : 0x7ffce24333f8
value of the pointer that points to a pointer that points to the first char of the array : 0x7ffce2433430
address of the pointer that points to the first char of the array : 0x7ffce2433430
value of the pointer that points to the first char of the array : 0x55a91985a2a0
address of the first char of the array: 0x55a91985a2a0
address of the seconds char of the array: 0x55a91985a2a1
value of the first char of the array : a
value of the second char of the array : b
value of the second char of the array : b
*str[1] is the same as *(str[1]), which runs to a segmentation fault
0x7ffce24333f8 0x7ffce2433430
---------------- ---------------- ----------------
| 0x7ffce2433430 | -------> | 0x55a91985a2a0 | -------> | a | 0x55a91985a2a0
---------------- ---------------- ----------------
----------------
| b | 0x55a91985a2a1
----------------
----------------
| c | 0x55a91985a2a2
----------------
The point is that it matters which pointer you dereference.
CodePudding user response：
At least this problem:
Off by one
str[count] = '\0'; can write outside array bounds leading to OP''s trouble. Suggest count < size --> count 1 < size.
Entire line not always read
Reading a partial line leads to trouble.
How about reading the entire line and report results? Let calling code provide the buffer as it is of fixed size.
Distinguish between reading an empty line and end-of-file.
Handle size == 0 gracefully.
// EOF: end-of-file with no input
// EOF: input error
// 0: input, but too much
// 1: Success
int secureInput(char *str, size_t size) {
if (str == NULL) {
size = 0;
}
bool too_many = false;
size_t count = 0;
int c;
while((c = getchar()) != '\n') {
if (c == EOF) {
if (feof(stdin) && count > 0) {
break;
}
if (size > 0) {
str[0] = '\0';
}
return EOF;
}
if (count 1 < size) {
str[count ] = c;
} else {
too_many = true;
}
}
if (count < size) {
str[count] = '\0';
}
return count < size && !too_many;
}