-1

I have written a program which sorts command line arguments. However, when I am trying to print the output(using a function), I am not able to do it.Because I am trying to pass char *[] in a function which accepts char** as argument. After a lot of searching, which resulted in nothing much, I am hence here with my first question in SO.

#include "iostream"
#include "cstdlib"
#include "cstring"
using namespace std;


void sortArgs();
int stringcomp (const void * x, const void * y);
void parse(char **argv, int argc);
void printArgs();
void setArgs(char **argv, int argc);

int size;
char** argNew;

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

parse(argv, argc);
printArgs();

return 0;

}

int stringcomp (const void *x, const void *y) 
{
    return strcmp (*(char * const *)x, *(char * const *)y);
}

void parse(char **argv, int argc)
{
   setArgs(argv, argc);
   sortArgs();
}


void setArgs(char **argv, int argc)
{
   argNew=argv;
   size=argc;
}

void  printArgs()
{

  char *s[size-1];
  cout<<size<<endl;

  for (int i = 1; i < size; i++)
  {
      s[i-1] = argNew[i];

  }

   for (int i = 0; i < size-1; i++)
        cout<<" "<< s[i];
   cout <<endl;
}

void sortArgs()
{


    int i;
    char *strings[size-1];

    /* assign each argument to a pointer */
    for (i = 1; i < size; i++)
    {
        strings[i-1] = argNew[i];
    }

    /* sort the array of pointers alphabetically with qsort */
    qsort (strings, size - 1, sizeof *strings, stringcomp);

    for (int i = 0; i < size-1; i++)
    cout<<" "<< strings[i];  //this prints the output just fine  


    setArgs(strings, size);    // pass the *strings[] here


 }

I am trying to pass strings in the function- setArgs() from the function sort(). Later when I use it to print the array, it gives me seg fault. Can anyone please help me visualize/rectify the problem here ?


PS: I understand that I can print the char* strings[] in the sort method itself, but my main focus is how to pass it to a function which accepts char** as argument.

sid chawla
  • 13
  • 3
  • 2
    A parameter of the type char* strings[] is adjusted to char **.:) – Vlad from Moscow Sep 14 '17 at 19:23
  • `char *s[size-1];` and `char *strings[size-1];` are Variable-Length Arrays, they are not standard in C++. – user7860670 Sep 14 '17 at 19:24
  • 3
    Fyi, you are saving the base address of an automatic object, `strings`, to your global `argNew`. Once `sortArgs` ends, that sequence of char pointers is gone, and so with it, defined behavior. – WhozCraig Sep 14 '17 at 19:25
  • @VladfromMoscow If you can see in my sortArgs() function, I tried passing it to the setArgs() function. But, when I try to iterate through it in my print function, I am not able to and rather it is giving me seg fault. – sid chawla Sep 14 '17 at 19:25
  • @WhozCraig's comment likely correctly explains the SEGV. The question is highly problematic because the code is just horrible. If you're asking about argument passing in C/C++ why are you storing values in globals instead of passing them? If you wanted to explore the argument passing, you could start with the simplest program that sorts and prints argv in main itself and then incrementally factor the sorting and printing into separate functions. The forward decls are unnecessary, etc. – Zalman Stern Sep 14 '17 at 19:33
  • When you call `setArgs` from `sortArgs` you just store the pointer to the local variable. So when you get to print this stored values - you got UB. You std::vectors, std::strings and std::sort – Artemy Vysotsky Sep 14 '17 at 19:36
  • @ZalmanStern I understand the code is really clumsy. What you are suggesting is "you could start with the simplest program that sorts and prints argv in main itself ", I have dont that already. But when I tried to use functions, I was not able to think anything else. If you could navigate me to a good reference to understand double pointers(**), it would really help me. – sid chawla Sep 14 '17 at 19:44
  • Would highly appreciate if while downvoting a question, people give the rationale too for doing so. Weren't you all a rookie at somepoint ?! – sid chawla Sep 14 '17 at 19:59
  • You have been given a rationale, which is that your question is confusingly unclear. At first it seems to be about sorting, argument passing, perhaps the difference between char ** and char *[], or maybe just "Why is my code segfaulting?" But it seems what you really want is suggestions on how to gain better insight into how pointers work. Perhaps starting off with "I am trying to better understand pointers in C..." and then asking for references or asking a direct question about the behavior of a small program would have worked better. Maybe try studying highly rated questions on SO a bit... – Zalman Stern Sep 15 '17 at 05:58

3 Answers3

1

You're assignment through setArgs from sortArgs is storing the base address of the automatic variable strings into a global. Once sortArgs exits and control is returned to parse, that array no longer exists. Any evaluation or dereference therein invoke undefined behavior.

You could fix this (term used loosely, see next section for why) by simply sticking with argv and argc set into your globals, and sort those, not some arguments passed in to some functions. Or you could copy the pointers to a pointer-bed like you're doing now, then sort them, then copy them back out to argvNew.

Or you would eliminate the globals entirely and throw out the set ideology in the process, simply having parse sort the pointers within argv directly You can't do much with it, but you can reorder it and still say in the realm of defined behavior.

But why? This is after, C++.

Jumping Into The C++ Pool

I'm not going to sugarcoat this. The code is dreadful. It is trying desperately to walk in the land of C++ using as little C++ standard library offerings as possible besides basic IO. C is a great language, and has a stellar, well-honed standard library, but if you want to dive into C++, then don't fear it; embrace it.

To do this in C++, consume some fundamental C++ standard library offerings. One could do things considerably differently. For example:

  1. Load a std::vector<std::string> from argv and argc
  2. Do whatever you want to that, including sorting it.

That code is far simpler than you may think:

#include <iostream>
#include <vector>
#include <string>

std::vector<std::string> parseArgs(char **argv, int argc)
{
    std::vector<std::string> args(argv+1, argv+argc);
    std::sort(args.begin(), args.end());
    return args;
}

int main (int argc, char** argv)
{
    auto args = parseArgs(argv, argc);

    // move to your own function.
    for (auto const& s : args)
        std::cout << s << '\n';

    return 0;
}
WhozCraig
  • 65,258
  • 11
  • 75
  • 141
  • I understand your point of view and highly appreciate your insight on this. But, what I am trying to do here is play a little with pointers. They always were a mystery to me. Using library, IMO, makes things simple but for a budding programmer, it makes his/her understanding superficial too. No offence ! – sid chawla Sep 14 '17 at 19:56
  • @sidchawla i completely understand; just don't assume the way things have always been done is routinely the way to continue to do them. C++ is *amazing*, and the standard library is just the bee's knees, especially from C++11 onward. Gain your understanding, but of many things, especially [RAII](http://en.cppreference.com/w/cpp/language/raii) concepts. It will change how you look at C++, and only for the better. – WhozCraig Sep 14 '17 at 22:48
0
setArgs(strings, size);    // pass the *strings[] here

That's the mistake.

strings is a local array. It gets deleted when the function returns. As a consequence, argNew will be a dangling pointer when the function returns. Trying to print the contents of argNew is undefined behavior.

Replace that line by copying the elements of strings to argNew.

for (i = 1; i < size; i++)
{
   argNew[i] = strings[i-1];
}
R Sahu
  • 204,454
  • 14
  • 159
  • 270
  • Thanks. Can you quote me a ref to learn more about ** pointers in general. I am trying to understand them for a while. – sid chawla Sep 14 '17 at 19:46
  • @sidchawla, I don't have any specific recommendations for understanding pointers. You might want to check out the [recommended books in SO](http://stackoverflow.com/questions/388242/the-definitive-c-book-guide-and-list). – R Sahu Sep 14 '17 at 19:51
0

There are many good searches such as "double pointers c" on StackOverflow or Google. E.g. c/c++ questions on pointers (double pointers) and http://www.cplusplus.com/doc/tutorial/pointers/ .

That said, the number one thing I'd suggest is that you can print a pointer's value just like a number. To do this either write:

printf("Pointer is %p.\n", ptr);

or:

std::cout << "Pointer is " << (void *)ptr << ".\n";

You can then analyze the output of the following:

#include <iostream>

// Sometimes it's char *, sometimes const char *
template <typename T>
void print_argv_style_array(const char *label, int argc, T **argv) {
    int i = 0;
    std::cout << label << ": Printing argv array of size " << argc << " at " << (void *)argv << ":\n";
    for (int i = 0; i < argc; i++) {
    std::cout << "    " << i << ": " << (void *)argv[i] << " -- " << argv[i] << "\n";
    }
}

void f1() {
    const char *strings[] = { "foo", "bar" , "baz" };
    print_argv_style_array("f1", sizeof(strings) / sizeof(strings[0]), strings);
}

void f2() {
    const char *strings[] = { "foo", "bar" , "quux" };
    print_argv_style_array("f2", sizeof(strings) / sizeof(strings[0]), strings);
}

const char *global_array[] = { "foo", "bar", "baz" };
const char **global_ptr = global_array;

int main(int argc, char **argv) {
    std::cout << "main: argv is at " << &argv << "\n";
    print_argv_style_array("main", argc, argv);
    f1();
    f2();
    std::cout << "global_array is at: " << (void *)global_array << "\n";
    print_argv_style_array("global_array", sizeof(global_array)/sizeof(global_array[0]), global_array);
    std::cout << "global_ptr is at: " << (void *)&global_ptr << "\n";
    print_argv_style_array("global_ptr", sizeof(global_array)/sizeof(global_array[0]), global_ptr);
}

Which on my system looks like:

main: argv is at 0x7fff5a8c38f0
main: Printing argv array of size 1 at 0x7fff5a8c3928:
    0: 0x7fff5a8c3ab8 -- ./pointer_help
f1: Printing argv array of size 3 at 0x7fff5a8c38a0:
    0: 0x10533dedc -- foo
    1: 0x10533dee0 -- bar
    2: 0x10533dee4 -- baz
f2: Printing argv array of size 3 at 0x7fff5a8c38a0:
    0: 0x10533dedc -- foo
    1: 0x10533dee0 -- bar
    2: 0x10533deeb -- quux
global_array is at: 0x10533e140
global_array: Printing argv array of size 3 at 0x10533e140:
    0: 0x10533dedc -- foo
    1: 0x10533dee0 -- bar
    2: 0x10533dee4 -- baz
global_ptr is at: 0x10533e158
global_ptr: Printing argv array of size 3 at 0x10533e140:
    0: 0x10533dedc -- foo
    1: 0x10533dee0 -- bar
    2: 0x10533dee4 -- baz

Notice that the locations of the strings arrays are the same for f1 and f2. (This is not guaranteed, it just happens to be the case here. It is certainly often true however and is useful to understand re: the SEGV bug you were seeing.) Note that string literals "foo", "bar", and "baz" get pooled -- that is there is only one copy of them even though they are used more than once. The sequence of string pointer values is consistent with the linker laying them out in order in memory as well.

Notice that &global_array is the same value as global_array, but this is not the case with global_ptr.

Zalman Stern
  • 3,161
  • 12
  • 18