0

I'm trying to create functions out of existing code in order to make it cleaner, and I'm having some problems:

It used to be:

int foo(char * s, char * t, char ** out) {
  int val = strcmp(s, t);
  if (val == 0) {
     *out = strdup(s);
     return 1;
  } else {
     *out = strdup(t);
     return 5;
  }
  return 0;
}

Now I have:

int foo(char * s, char * t, char ** out) {
  someFunction(s, t, out);
  printf("%s", *out);
  return 0;
}

int someFunction(char *s, char * t, char **out) {

  int val = strcmp(s, t);
  if (val == 0) {
     *out = strdup(s);
     return 1;
  } else {
     *out = strdup(t);
     return 5;
  }
  return 0;
}

And I'm getting segmentation faults when I try to do the printf. Should someFunction be expecting a *out? I guess I'm still confused.

Rio
  • 14,182
  • 21
  • 67
  • 107
  • 7
    What does `someFunction` look like? Also, that can't be the code you compiled and ran, since it's missing a function name. Please paste the real code (culled for brevity, of course). – Marcelo Cantos Apr 11 '11 at 22:41
  • 1
    It would also be helpful to see how you are calling the (currently nameless) function. Specifically, what are you passing in for values `s`, `t`, and `out`? A complete, compilable example that reproduces your segfault would be ideal. – kqnr Apr 11 '11 at 22:44
  • I tried to fix it as best I can. – Rio Apr 11 '11 at 22:49

2 Answers2

2

This code is "correct" if I understand your intent. I assume you are doing something along the lines of

char *s = "foo";
char *t = "bar";
char *out;
foo(s, t, out);

when you really want

char *s = "foo";
char *t = "bar";
char *out;
foo(s, t, &out);  // Note the & which passes the address of a char* to be manipulated
kqnr
  • 3,596
  • 19
  • 17
  • No, `foo` is your `foo`. You are calling `foo` somewhere. Your call to `foo` must be incorrect (as in my first example), because there is nothing wrong with the way `foo` is calling `someFunction`, or with the way that `someFunction` manipulates `*out`. – kqnr Apr 11 '11 at 22:57
  • In other words, you are providing an invalid `char**` for the `out` argument of `foo`. This ultimately causes a segfault when `someFunction` attempts a write to the invalid memory address. – kqnr Apr 11 '11 at 22:58
  • So even if foo calls someFunction, ultimately out is always accessible with the right content? – Rio Apr 11 '11 at 23:05
  • 1
    I'm not sure what you mean by "always accessible." `out` is a pointer to a pointer. You pass it from `foo` over to `someFunction`. There is nothing wrong with the way you are doing this in the code you've provided. It is correct code. Your segfault *must* mean that your problem is in how you call `foo`. You have not posted that code though, so it cannot be analyzed. – kqnr Apr 11 '11 at 23:12
  • Wouldn't this normally trigger a compiler error since there is no explicit cast from `char*` to `char**`? I would try to compile with `gcc -Wall` and see if it fails to compile pending some incorrect casts. Typically I would imagine gcc might warn you without the `-Wall` option, and just output a list of errors. With the `-Wall` option though, it would stop you from incorrect pointer casts if that is indeed what's happening. – Jason Apr 12 '11 at 04:01
0

I would convert it this way:

int foo(char * s, char * t, char ** out) {
    int val = strcmp(s, t); 
    *out = val ? strdup(t) : strdup(s);
    return val ? 5 : 1;
}
pajton
  • 15,828
  • 8
  • 54
  • 65
  • While this may be an acceptable way to refactor the function `foo` (though I would disagree as it tests `val` twice), it does not actually answer the question that was asked. – kqnr Apr 11 '11 at 22:55
  • A note of thanks though, every attempt asking a question here is a) preceded by at least half an hour of head-banging-against-table and then followed by a zen-like appreciation for newfound knowledge. – Rio Apr 11 '11 at 22:59