0

I'm having trouble storing an integer into an array of strings using sprintf(). I am trying to create a new argv list to pass into my child process. I have 'curr' storing the correct value since I've tested in in GDB. My code is as follows:

#include <string.h>
#include <stdlib.h>
#include <stdio.h>
#include <unistd.h>
#include <signal.h>
#include <sys/types.h> /* for pid_t */

int main(int argc, char *argv[]){

static char *argv2[] = {"./datagen", "10", "outputfile", "SIGUSR1"};

pid_t pid = fork();
int curr = getpid();
sprintf(argv2[4], "%s", curr);


if(pid == 0)
{
    printf("You are in the child process.\n");

}
else{
    printf("You are in the parent process.  Process ID is %d\n", getpid());
}


return;
}

After exhaustively searching around for a clear answer, I have yet to find anything. Ideally, the 4th slot of argv2 will store the process id as a string. However, I am getting a segmentation fault 11. If anyone could shed some light on this issue I would be eternally grateful.

Thank you!!

1
  • char buf[sizeof curr * CHAR_BIT/3 + 3]; sprintf(buf, "%d", curr); Commented Jan 20, 2015 at 16:52

4 Answers 4

4

You can't do that

static char *argv2[] = {"./datagen", "10", "outputfile", "SIGUSR1"};

is a declaration of an array of char pointers, which are pointing to string literals, and further more to four string literals only, you can't extend it nor modifiy the strings.

What you need is

char argv2[10][100] = {"./datagen", "10", "outputfile", "SIGUSR1"};

assuming that you want 10 strings of maximum length 100, which you can obviously change.

Also, the format specifier for integers is "%d" so you have another mistake, having said all that you can now

sprintf(argv2[4], "%d", curr);

and I would suggest the snprintf() function, since it will avoid buffer overflow problems,

snprintf(argv2[4], sizeof(argv[4]), "%d", curr);

chux comment is correct if you want to have control on whether the specified length of the string was enough, you should check the return value of snprintf(), in case there wasn't sufficient space to write all the source string into the destination it will be truncated, if snprintf() returns a value larger or equal to the requested maximum, it means that the string was truncated, so a simple check like

if (snprintf(argv2[4], sizeof(argv[4]), "%d", curr) >= sizeof(argv[4]))
    doSomething_TheString_Was_Truncated();

although, for 100 characters and the "%d" that will not happen, but I firmly believe that people must write safe code as a habit, rather than only checking possible problems, check for every thing that can conceptually go wrong, no matter how unlikely. Because sometimes there will be situations where an unexpected thing will happen.

Note: as chux pointed out again, snprintf() will return a negative in case of an error, you can check for that separately, to check if there was an error.

Sign up to request clarification or add additional context in comments.

2 Comments

Using snprintf() vs. sprintf() does prevent buffer overflow problems but trades an overflow problem with another. Unless the return value of snprintf() is checked, the truncated result in argv2[4] is indistinguishable from valid results. Thus later code will operate with an invalid value in the string. IMO, an unchecked snprinf() provides a false sense of protection. Recommend showing how to check the result.
Nice improvement.. Pedantic point: snprintf() return value could be negative "if an encoding error occurred."
1

Simple steps:
1) Determine array size information from existing argv, argc arguments.

int i, len=0, lenKeep=21;//initialize large enough to contain pid integer
for(i=0;i<argc;i++)
{
    len = strlen(argv[i])
    if(lenKeep<len)lenKeep = len;
}  

2) use that size information to create a new string array, argv2, with additional elements if necessary. (argv2 will be an array of strings, create sufficient space.)

char argv2[argc+1][lenKeep+1];    
//argc+1 allows for additional array element
//lenKeep+1 provides space for all existing content  

3) add new information to the string array in the normal way.

sprintf(argv2[argc], "%d", curr); //argv2 array contains argc + 1 elements 
                                  //so now argc is a valid index value

5 Comments

@chux - Yeah, you beat me to the punch by a few seconds. Good catch. Thanks
Suggest lenKeep = MAX_INT_LENGTH like lenKeep = 21, for it is not known the strlen(argv[i]) is big enough for the string version of curr.
I don't know, but I find this style if(lenKeep<len)lenKeep = len; really confusing, any reason you could give for using that? I would like to learn if there is a good reason, since I see it very often.
It makes sense, but it's ugly.
@iharob - Lol!. If this syntax is ugly to you, I suspect you will avoid using it . :)
0

sprintf(argv2[4], "%s", curr); breaks the array, which has only 4 elements.

Even if it has more elements, you are writing to a string literal which is Undefined Behaviour.

Comments

0

Try using %d instead of %s, it should anyways be saved as a C string.

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.