Home > other >  Best way to modify a string within a function?
Best way to modify a string within a function?

Time:12-11

So I have a sample program below (part of a larger program), and I need to pass in a pointer to a string (double pointer for char) to a function, and modify the string within the function. What is the best way to accomplish this?

#include <string.h>
#include <stdio.h>
int incr(char **ptr)
{
   char ar[104];
   scanf("%s\n",ar);
   *ptr = ar;
   // this prints the string correctly
   printf("%s\n",*ptr);
   return 0;
}

int main(void)
{
   char *d;
   // pass the string (char array) to function
   // expecting the input from scanf to be stored
   // in this pointer (modified by the function)
   incr(&d);
   printf("%s\n",d);
   return 0;
}

Output from valgrind:

$ gcc test.c -o terst
$ valgrind --tool=memcheck --leak-check=yes --show-reachable=yes ./terst
==1346438== Memcheck, a memory error detector
==1346438== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==1346438== Using Valgrind-3.16.0 and LibVEX; rerun with -h for copyright info
==1346438== Command: ./terst
==1346438==
Sampletexttodisplay
Sampletexttodisplay
==1346438== Conditional jump or move depends on uninitialised value(s)
==1346438==    at 0x4C38329: strlen (vg_replace_strmem.c:459)
==1346438==    by 0x4EB48D5: puts (in /usr/lib64/libc-2.28.so)
==1346438==    by 0x400658: main (in prog/terst)
==1346438==
==1346438== Conditional jump or move depends on uninitialised value(s)
==1346438==    at 0x4C38338: strlen (vg_replace_strmem.c:459)
==1346438==    by 0x4EB48D5: puts (in /usr/lib64/libc-2.28.so)
==1346438==    by 0x400658: main (in prog/terst)
==1346438==
==1346438== Conditional jump or move depends on uninitialised value(s)
==1346438==    at 0x4EBE86D: _IO_file_xsputn@@GLIBC_2.2.5 (in /usr/lib64/libc-2.28.so)
==1346438==    by 0x4EB4992: puts (in /usr/lib64/libc-2.28.so)
==1346438==    by 0x400658: main (in prog/terst)
==1346438==
==1346438== Conditional jump or move depends on uninitialised value(s)
==1346438==    at 0x4EBE87F: _IO_file_xsputn@@GLIBC_2.2.5 (in /usr/lib64/libc-2.28.so)
==1346438==    by 0x4EB4992: puts (in /usr/lib64/libc-2.28.so)
==1346438==    by 0x400658: main (in prog/terst)
==1346438==
==1346438== Syscall param write(buf) points to uninitialised byte(s)
==1346438==    at 0x4F2F648: write (in /usr/lib64/libc-2.28.so)
==1346438==    by 0x4EBE1FC: _IO_file_write@@GLIBC_2.2.5 (in /usr/lib64/libc-2.28.so)
==1346438==    by 0x4EBD56E: new_do_write (in /usr/lib64/libc-2.28.so)
==1346438==    by 0x4EBF2B8: _IO_do_write@@GLIBC_2.2.5 (in /usr/lib64/libc-2.28.so)
==1346438==    by 0x4EBF692: _IO_file_overflow@@GLIBC_2.2.5 (in /usr/lib64/libc-2.28.so)
==1346438==    by 0x4EB4A61: puts (in /usr/lib64/libc-2.28.so)
==1346438==    by 0x400658: main (in prog/terst)
==1346438==  Address 0x5207490 is 16 bytes inside a block of size 1,024 alloc'd
==1346438==    at 0x4C34F0B: malloc (vg_replace_malloc.c:307)
==1346438==    by 0x4EB260F: _IO_file_doallocate (in /usr/lib64/libc-2.28.so)
==1346438==    by 0x4EC04BF: _IO_doallocbuf (in /usr/lib64/libc-2.28.so)
==1346438==    by 0x4EBF727: _IO_file_overflow@@GLIBC_2.2.5 (in /usr/lib64/libc-2.28.so)
==1346438==    by 0x4EBE8CE: _IO_file_xsputn@@GLIBC_2.2.5 (in /usr/lib64/libc-2.28.so)
==1346438==    by 0x4EB4992: puts (in /usr/lib64/libc-2.28.so)
==1346438==    by 0x400631: incr (in prog/terst)
==1346438==    by 0x40064C: main (prog/terst)
==1346438==
)▒▒lay
==1346438==
==1346438== HEAP SUMMARY:
==1346438==     in use at exit: 0 bytes in 0 blocks
==1346438==   total heap usage: 2 allocs, 2 frees, 2,048 bytes allocated
==1346438==
==1346438== All heap blocks were freed -- no leaks are possible
==1346438==
==1346438== Use --track-origins=yes to see where uninitialised values come from
==1346438== For lists of detected and suppressed errors, rerun with: -s
==1346438== ERROR SUMMARY: 40 errors from 5 contexts (suppressed: 0 from 0)
$

As you can see, the printf in main doesn't print the expected output "Sampletexttodisplay" (it just outputs a bunch of garbage) while the printf within the incr function does. So something happens that the original pointer gets modified, but not to the desired string. Is there quick fix for this or is there some more preferred method for modifying strings within functions? Thanks for help.

CodePudding user response:

d is already a pointer, you can directly use it. But first, you need to allocate some memory to it with malloc().

Also, scanf("%s\n",d) should not be used, the newline at the end will make it so that scanf() fill forever wait for input. The typed newline at the end is automatically removed while using scanf(). Instead just use scanf("%s",d).

Working code:

#include <string.h>
#include <stdio.h>
#include <stdlib.h>

int incr(char *ptr)
{
   scanf("%s",ptr);
   printf("%s\n",ptr);
   return 0;
}

int main(void)
{
   char *d = malloc(sizeof(char) * 104);
   // pass the string (char array) to function
   // expecting the input from scanf to be stored
   // in this pointer (modified by the function)
   incr(d);
   printf("%s\n",d);
   free(d);
   return 0;
}

CodePudding user response:

As suggested by @RetiredNinja in comments, the problem lies with the lifespan of the array ar. This array is allocated on the stack for the duration of the call to incr. After incr returns, a pointer to this memory location may work, but it may not.

If the memory is dynamically allocated, its lifespan is not bound to the function call, and it will remain valid until it is freed with free.

You are also well-advised to specify a field width in scanf.

int incr(char **ptr)
{
   char *ar = malloc(104); // char is 1 byte, so this allocates 104 chars

   scanf("3s", ar);
   *ptr = ar;

   printf("%s\n", *ptr);
   return 0;
}

Alternately, there is no need for this, and we can use scanf to read text directly into ptr.

int incr(char **ptr)
{
   scanf("3s", *ptr);

   printf("%s\n", *ptr);
   return 0;
}

This has two advantages. It avoids this function allocating memory dynamically, which we might lose track of. It also means that any memory initially allocated for ptr is not leaked.

However, it deprives us of an opportunity to scan the text in and validate it before changing ptr.

CodePudding user response:

You succeeded "to modify a string within a function". Well not actually modify, more like create via printing. You can output it within the function, while the array you stored the string in is still valid and accessable, i.e. right until the end of the function execution.

After that it is not allowed anymore to access the now non-existing local variable anymore. If you would not do that everything would be fine - apart from a slightly weird API which allows to keep a pointer to something forbidden outside of the funcition. Your pointer to poniter construct in this way is very similar to returning a forbidden pointer from a function.

Then you DO access the forbidden, not-anymore-existing string. And that fails, of course.

You can either hand in a parameter of a pointer (in my opinion not a pointer to pointer, which is unneeded) to a string or at least to legal memory; that would have to be part of your interface definition "Dear user, the pointer you hand in has to point to valid and large enough legal memory for a string."

Or you can stick with your pointer to pointer interface and define that the pointer afterwards will point to something suitable. For that you would have to us a legal way of creating such legal memory, e.g. malloc(). And your interface specification would have to include "Dear user, the pointer referenced by the pointer-to-pointer parameter will afterwards poin to legal memory. If you call this function, you are responsible for correctly freeing it at the end of using it."

CodePudding user response:

A few things: From your comment, it seems like you

You're not trying to modify the pointer, you're trying to modify what it's pointing to, so you can just pass the original pointer.

Where your code fails is, as Retired Ninja points out in his comment, your original character array is destroyed when you leave your function, since it's a local variable. To avoid this, you should declare it in main.

I have written what I believe you intended to do below. Here, we can see the following is happening:

  1. You create the space for your array with char ar[104]. Here ar is pointing to your array
  2. You pass ar to your function incr where a local copy of it is created and stored as ptr. But since we aren't trying to change the pointer, but rather what it's pointing at, that's ok.
  3. The data is written to the place ptr is pointing to which is the same place our original ar pointer is pointing to.
#include <string.h>
#include <stdio.h>

int incr(char* ptr)
{
    scanf("%s\n",ptr);
    printf("%s\n",ptr);
    return 0;
}
int main(void)
{
    char ar[104];
    incr(ar);
    printf("%s\n",d);
    return 0;
}
  • Related