0

I'm reading Brian W Kernighan and Dennis M Ritchie The C Programming Language, 2nd Edn and practicing using function pointers. I've implemented one of the examples from the book. I kept getting a segfault when addressing some of the functions.

After stepping through it with gdb I realized that when passing a function address to a pointer the pointer remained null, but only for some of the functions. Adding the & operator to the function names causing the error fixed the issue.

I've read everywhere that functions can be addressed by their name with or without the & operator. I cannot figure out why these functions required it. Or could it have something to do with the way I was assigning it?

EDIT: I understand the code is quite buggy. I was just doing a quick exercise from an old book. I will work on minimizing it. I'm just confused why when I step through the program with gdb fp remains null (without the &)after the assignment on lines 20 and 25 while on lines 27 and 29 fp does take get assigned the addresses.

  #include <stdio.h>
  #include <string.h>
   
  #define MAXLINES 5000
  char *lineptr [MAXLINES];
   
  int readlines(char *lineptr[], int nlines);
  void writelines(char *lineptr[], int nlines);
   
  void my_qsort(void *lineptr[], int left, int right, int (*comp)(void *, void *));
  int numcmp(char *, char *);
  int reva(char *, char *);
  int revn(char *, char *);
  
  
  int main(int argc, char **argv) {
  
          int nlines;
          int numeric = 0;
          int (*fp)(void*, void*) = &strcmp;
  
          if(argc > 1) {
                  for(int i = argc; i > 1; i--) {
                          if(strcmp(argv[i-1], "-n") == 0)
                                  fp = &numcmp;
                          else if(strcmp(argv[i-1], "-r") == 0)
                                  fp = reva;
                          else if(strcmp(argv[i-1], "-rn") == 0 || strcmp(argv[i-1], "-nr") == 0)
                                  fp = revn;
                          else {
                                  printf("error: invalid modifier [%s]\n", argv[i-1]);
                                  return -1;
                          }
                  }
          }
          if((nlines = readlines(lineptr, MAXLINES)) >= 0) {
                  my_qsort((void **) lineptr, 0, nlines-1, fp);
                  writelines(lineptr, nlines);
                  return 0;
          } else {
                  printf("input too big to sort\n");
                  return 1;
          }
  }
  
  
  void my_qsort(void *v[], int left, int right, int (*comp)(void *, void *)) {
  
          int i, last;
          void swap(void *[], int, int);
  
          if (left >= right)
                  return;
          swap(v, left, (left+right)/2);
          last = left;
          for(i = left+1; i <= right; i++)
                  if((*comp)(v[i], v[left]) < 0)
                          swap(v, ++last, i);
          swap(v, left, last);
          my_qsort(v, left, last-1, comp);
          my_qsort(v, last+1, right, comp);
  
  }
  
  
  #include <stdlib.h>
  
  int numcmp(char *s1, char *s2) {
  
          double v1, v2;
  
          v1 = atof(s1);
          v2 = atof(s2);
          if(v1 < v2)
                  return -1;
          else if(v1 > v2)
                  return 1;
          else
                  return 0;
  }
  
  
  int reva(char *s1,char *s2) {
          return -strcmp(s1, s2);
  }
  
  
  int revn(char *s1, char *s2) {
          return -numcmp(s1, s2);
  }
  
  void swap(void *v[], int i, int j) {
  
          void *temp;
  
          temp = v[i];
          v[i] = v[j];
          v[j] = temp;
  }

lines.c

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

#include "get_line.h"

#define MAXLEN 1000 // max length of any input line
char *alloc(int);

// readlines: read input lines
int readlines(char *lineptr[], int maxlines) {

        int len, nlines;
        char line[MAXLEN];
        char lines[maxlines*MAXLEN];
        char *plines = lines;

        nlines = 0;
        while((len = get_line(line, MAXLEN)) > 0)
                if(nlines >= maxlines)
                        return -1;
                else {
                        line[len-1] = '\0'; // delete newline
                        strcpy(plines, line);
                        lineptr[nlines++] = plines;
                        plines += len;
                }
        return nlines;
}


// writelines: write output lines
void writelines(char *lineptr[], int nlines) {

        int i;

        for(i = 0; i < nlines; i++)
                printf("%s\n", lineptr[i]);
}

get_line.c

#include <stdio.h>
#include "get_line.h"

//getline: get line into line[]; return length
int get_line(char line[], int lim) {

        int c, i;

        i = 0;
        while(--lim > 0 && (c = getchar()) != EOF && c != '\n') {
                line[i++] = c;
        }
        if(c == '\n') {
                line[i++] = c;
        }
        line[i] = '\0';

        return i;
}
  • 3
    Please don't include line numbers — we have to remove them to compile your code, which makes work for those who might help you. By all means indicate which line is which, but we can't simply copy and paste your code. – Jonathan Leffler Sep 01 '22 at 01:29
  • You're calling functions that take `char *` or `const char *` through function pointers that say they take `void *`. – user2357112 Sep 01 '22 at 01:31
  • You load up to 5000 "lines", then possibly invoke `atof()` on pairs of lines to compare their values... That will certainly warm up the CPU... – Fe2O3 Sep 01 '22 at 01:34
  • 2
    You should pay attention to compiler warnings: https://godbolt.org/z/4vdd9aar4 `initialization of 'int (*)(void *, void *)' from incompatible pointer type 'int (*)(const char *, const char *) [-Wincompatible-pointer-types]` and `assignment to 'int (*)(void *, void *)' from incompatible pointer type 'int (*)(char *, char *)' [-Wincompatible-pointer-types]` point to the problem. – Jerry Jeremiah Sep 01 '22 at 01:36
  • You source isn't compilable because `readlines` isn't included. It would help if we had a compileable example. – Jerry Jeremiah Sep 01 '22 at 01:38
  • 1
    I get multiple warnings like: `initialization of ‘int (*)(void *, void *)’ from incompatible pointer type ‘int (*)(const char *, const char *)’ [-Werror=incompatible-pointer-types]` for the line `int (*fp)(void *, void *) = &strcmp;` and similar ones. It is OK to cast a pointer to a function to a different function pointer type as long as the type is converted back to the declared type before the function is invoked. You'd probably do better, though, making the comparators take two `const void *` arguments and then inside the functions converting the pointers passed to the correct type. – Jonathan Leffler Sep 01 '22 at 01:39
  • You probably crashed with `strcmp()` because you can't pass `strcmp()` to the system `qsort()` for sorting arrays of strings. The types of the arguments are wrong (you're getting `char **` cast to `void *`, not `char *`). I think you run into the same problem here. You also are playing dubious games with the `void *v[]` argument to your `my_qsort()` function; the standard uses `void qsort(void *base, size_t nel, size_t width, int (*compar)(const void *, const void *));` — which is noticeably different. – Jonathan Leffler Sep 01 '22 at 01:44
  • The answer to your headline question is "it should make no difference whether you use an `&` or not". Even macro overloads for system functions should not give problems. – Jonathan Leffler Sep 01 '22 at 01:45
  • 4
    your issue has nothing to do with `&`. The crash is caused by some other issues in your program. You need to debug the crash. run a debugger, and/or valgrind on your program. – Serge Sep 01 '22 at 02:23
  • @Jonathan Leffler sorry, I've edited out the line numbers. Most of this code is an example in the book that I've edited. I understand the book is quite old. The code that casts void* to char* is directly from the book. I took the compiler warning as just a warning since the book (in my understanding) basically states that is the purpose of void*, to make a generic pointer. – user3143158 Sep 01 '22 at 03:40
  • @Jerry Jeremiah I've added the rest of the source. – user3143158 Sep 01 '22 at 03:44
  • The C standard ([§6.2.3.3 (Conversions of) Pointers ¶8](http://port70.net/~nsz/c/c11/n1570.html#6.3.2.3p8) says: _A pointer to a function of one type may be converted to a pointer to a function of another type and back again; the result shall compare equal to the original pointer. If a converted pointer is used to call a function whose type is not compatible with the referenced type, the behavior is undefined._ It means that code abusing function pointers like this is not guaranteed to behave well. (K&R 2nd Edn has problems related to function pointers being misused.) OTOH, it may work OK. – Jonathan Leffler Sep 01 '22 at 03:44
  • 1
    You are returning pointers to local variables from `readlines`. This cannot work. – n. m. could be an AI Sep 01 '22 at 03:46
  • I understand the code is quite buggy. I was just doing a quick exercise from an old book. I'm just confused why when I step through the program with gdb `fp` remains null (without the `&`)after the assignment on lines 20 and 25 while on lines 27 and 29 `fp` does take get assigned the addresses. – user3143158 Sep 01 '22 at 03:52
  • Please read [mre] and https://stackoverflow.com/questions/25385173/what-is-a-debugger-and-how-can-it-help-me-diagnose-problems. – Karl Knechtel Sep 01 '22 at 03:53
  • Your program has *undefined behaviour*. You cannot reason about it. It can do literally anything. – n. m. could be an AI Sep 01 '22 at 04:06
  • 1
    If you want to know specifically what happens at line 25, then your program is very far from being minimal. Please remove everything that is not needed for reproducing the problem. All the (buggy) sorting reading and writing are irrelevant. – n. m. could be an AI Sep 01 '22 at 04:10
  • @Karl Knechtel I've stated the exact result I am getting from the debugger and on what line. My question pertains to my debugging results. I will update the question to make this more clear. Also, I was asked in a previous comment to provide the full code. Sorry it's a bit long, I will do better cutting it down next time. – user3143158 Sep 01 '22 at 04:18
  • "Also, I was asked in a previous comment to provide the full code." I don't see such a comment here. You were asked for *a compilable example*, in other words a [mre], multiple times. – Karl Knechtel Sep 01 '22 at 04:20
  • Well I feel stupid. I just spent a bunch of time trying to minimize the code and reproduce the same result in gdb but couldn't reproduce it (it was working now without the & on lines 20 and 25). So I went back to the full source to delete the &'s to make sure it would still fail but now it was working as it should. Not sure what was happening before but I feel bad for wasting everyone's time. Thanks for your help anyways. I guess I got my answer though, that there is no special case where the & operator is necessary when getting the address of a function. – user3143158 Sep 01 '22 at 04:58
  • @user3143158 The reason it appears to work now and it didn't before is because of the undefined behaviour. Literally anything can happen - the effect of the code could change between iterations of a loop (if the loop is unrolled by the optimizer) or it could change when you call a function that you just called. Literally anything can happen - even time travel: https://devblogs.microsoft.com/oldnewthing/20140627-00/?p=633 You need to fix the undefined behaviour or it will randomly stop working again in a new, interesting way. – Jerry Jeremiah Sep 01 '22 at 21:04

0 Answers0