0

I'm starting off in C programming, and I'm currently studying structures and using pointers to work with them. I've been trying to write a simple program which stores your personal details (name and birthday) for practice, but I've been having trouble with memory allocation and I'm clearly missing something - so I tried something simpler, and apparently I'm having trouble with allocating memory for the string. I've tried my best at debugging this, but I've no clue what's wrong. 

#include<stdio.h>
#include<stdlib.h>
#include<stddef.h>
#include<string.h>
int main()
{
    char DummyString[100];
    printf("Enter a string to be read back to you: ");
    scanf("%s",&DummyString);
    printf("The string is: %s\n",DummyString);
    printf("String length is %d\n",strlen(DummyString));
    printf("sizeof char is %i\n",sizeof(char*));
    // char DumbStringPtr=&DumbString;
    char *DummyStringPtr = (char*)malloc((sizeof(char))*(strlen(DummyString)+1)); // returns 8 regardless of anything
    printf("The size of the pointer would be %d\n", sizeof(DummyStringPtr));
}

In the original program, the failure was after trying to copy a string into the allocated pointer, which I'm assuming is due to shortage of memory (will show code if necessary). I feel like I've missed something fundamental and so would appreciate any feedback. I'd really appreciate it if someone can give me a lead on my mistake. Thanks!

dkd6
  • 65
  • 1
  • 6
  • 1
    The question is unclear to me. What do you mean by "// returns 8 regardless of anything" ? What is returning 8 ? `malloc` ? `strlen` – Support Ukraine Apr 14 '20 at 09:04
  • 1
    `scanf("%s",&DummyString);` where `DummyString` is already a pointer (array/pointer conversion), so no `'&'` before `DummyString`... You cannot use an user-input function correctly without ***checking the return***. (most important lesson of the night) – David C. Rankin Apr 14 '20 at 09:05
  • This line: `printf("sizeof char is %i\n",sizeof(char*));` is strange. The printed text says **sizeof char** but you take sizeof **char pointer**. Notice that `sizeof(char)` is always 1 – Support Ukraine Apr 14 '20 at 09:09
  • Hi, thanks for the input! my intention was to try and track how much memory I'm allocating, but I just returned the size of the pointer. The rest (like sizeof(char) are frantic attempts at debugging :) – dkd6 Apr 14 '20 at 09:13
  • regarding: `scanf("%s",&DummyString);` 1) a 'bare' array name degrades to the address of the first byte of the array. Placing a `&` before the array name will cause the compiler to output a warning message, Not what you want. 2) when using the 'input format conversion' specifiers `%s` and/or `%[...]` always include a MAX CHARACTERS modifier that is 1 less than the length of the input buffer because these specifiers always append a NUL byte to the input. This also avoids any chance of a buffer overflow and the attendant undefined behavior (cont) – user3629249 Apr 16 '20 at 02:03
  • (cont) the `scanf()` family of functions return the number successful conversions. (or EOF) so in the current scenario, any returned value other than 1 indicates an error occurred. – user3629249 Apr 16 '20 at 02:04
  • the posted code contains a memory leak because the memory allocated via the call to `malloc()` was not returned to the system via a call to `free()` – user3629249 Apr 16 '20 at 02:06

1 Answers1

3
  • scanf("%s",&DummyString); is wrong. Since DummyString is an array, you get a pointer to its first element when you use it in an expression, and that's what scanf expects. Change to scanf("%s",DummyString);.
  • The malloc line is needlessly bloated, simply do
    char *DummyStringPtr = malloc(strlen(DummyString)+1));
  • The size of the pointer is always 4 (or 8 etc) regardless of where it points. sizeof will not tell you how much memory you have allocated. You have to keep track of this yourself, in a separate size variable if needed.
  • Correct printf conversion specifier for the result of sizeof is %zu since sizeof returns type size_t. Quick & dirty version for newbies is to use printf("%d", (int)sizeof(something));
  • Always free(DummyStringPtr);.
Lundin
  • 195,001
  • 40
  • 254
  • 396
  • @TomKarzes Needlessly pedantic for newbies, but fair enough. – Lundin Apr 14 '20 at 09:06
  • Of course! these all make sense. Going to try editing according to this. Thank you! How can I track the amount of memory I've allocated? – dkd6 Apr 14 '20 at 09:08
  • @dkd6 Simply do something like `int size = strlen(dummyString)+1; ... malloc(size);`. (Again, pedantically the correct integer type is actually `size_t` not `int`, but newbies need to worry about that yet) – Lundin Apr 14 '20 at 09:14
  • `if (scanf("%99s", DummyString) != 1) { ... }` always use the *field-width* modifier to protect your array bounds (otherwise `scanf()` is no better than `gets()`) and always check the return (you covered the rest) – David C. Rankin Apr 14 '20 at 09:15
  • Fantastic! thank you all for helping a newbie out :) I've a lot to apply now! – dkd6 Apr 14 '20 at 09:16
  • @Lundin Using an `int` to hold a `sizeof` value is fine. It's only a problem if the value is too large to fit into an `int`. The incorrect format case is *very* different, and *not* pedantic. If `sizeof(size_t)` is larger than `sizeof(int)`, then using `%d` format will *not work*, even if the `sizeof` value is small. Remember, it's not the value that matters, it's the size of the *data type*. Yes, even `printf("%d\n", sizeof(int));` will fail on some platforms. So, no, not pedantic. It's a portability issue. – Tom Karzes Apr 14 '20 at 09:25
  • @Lundin But casting a `sizeof` value to an `int` is fine (as long as you know the value isn't huge), e.g. `printf("%d\n", (int) sizeof(int));` is fine. – Tom Karzes Apr 14 '20 at 09:42
  • @DavidC.Rankin If you ask me, then the correct practice is rather to never use `stdio.h` in production-quality code to begin with. – Lundin Apr 14 '20 at 09:46
  • @TomKarzes Mhm, may or may not work depending on endianess. Big endian machines would print zero or something. But for fully portable code, you shouldn't be using `int` either, but the types from `stdint.h`. – Lundin Apr 14 '20 at 09:50
  • @Lundin Right, that was my point. And even if it picked up the low-order portion of the value, it may still consume more "argument" than it should, and could throw off the following argument (if any). But if you cast to `(int)`, then the only issue is whether the actual value can fit into an int. Certainly it should for any primitive data type, and any sane structures that don't contain arrays. It's when arrays are present that you run the risk of having an extremely large value that requires the added range of a `size_t`. – Tom Karzes Apr 14 '20 at 09:54
  • @Lundin Also, the different integer sizes may have different alignments, which could also prevent it from extracting the low-order potion of the value when expecting an `int`. – Tom Karzes Apr 14 '20 at 09:57
  • @Lundin, sorry the comment was for the benefit of the OP -- I know you know. I'm curious though, If we don't use `stdio.h` in production code -- what do we use in its place? Without it, we are pretty much left with `read/write` which somehow seem woefully inadequate. – David C. Rankin Apr 14 '20 at 15:43
  • @DavidC.Rankin Depends on the system. I mostly work with embedded so maybe I'm biased. For desktop systems, people stopped writing console applications some 20 years ago. But if you _must_ use console input for some strange reason (rather than just using argv argc or pipes etc), then I'd use the platform-specific console API directly. – Lundin Apr 15 '20 at 07:47