1

While reading Unix System Design by Maurice Bach I came across below code snippet.

#include < signal.h>
char *cp;
int callno;

main() {
    char *sbrk();
    extern catcher();

    signal(SIGSEGV, catcher);
    cp = sbrk(O);
    printf("original brk value %u\n", cp);
    for (;;)
    *cp++ = 1; 
}


catcher(signo) {
    int signo;
    callno++;
    printf("caught sig %d %dth call at addr %u\n", signo, callno, cp);
    sbrk(256);
    signal(SIGSEGV, catcher); 
}

I got confused with two statements within main method

char *sbrk();

extern catcher();

I understand how extern works and I also know what sbrk() does but I couldn't understand why have they written extern before catcher() and also why is char* written before sbrk() call?

I got compilation error on gcc-4.8.4 on Ubuntu when compiling this code but code compiles without any errors in Mac. Why is this happening?

Community
  • 1
  • 1
Nullpointer
  • 1,086
  • 7
  • 20
  • 2
    [You are reading something wrote in 1986........](http://160592857366.free.fr/joe/ebooks/ShareData/Design%20of%20the%20Unix%20Operating%20System%20By%20Maurice%20Bach.pdf)You should buy recent books that, at least, support c99 – LPs May 03 '17 at 13:37
  • In older versions of C, there was a concept of "default type" - if the return type of function or the type of a variable was not specified, it was assumed to be `int`. – kfx May 03 '17 at 13:37
  • @LPs I understand that this a very old book but I just want to know what it means. @kfx How is that related to `extern` of `cather()` or `char *` of `sbrk()` – Nullpointer May 03 '17 at 13:40
  • the `extern` is used to avoid declaring a prototype at the top of the file, I guess. **It is useless as the book where the code is written.** – LPs May 03 '17 at 13:41
  • 1
    The `extern` is there because you need _something_ to make it a function declaration rather than a function call. In this case, if default function return types are supported, it is equivalent to `int catcher();`. – Ian Abbott May 03 '17 at 13:41
  • It appears as it is just decalring these functions before they are called, may be thats what older compilers expected – Pras May 03 '17 at 13:41
  • The syntax of the `catcher` function is wrong. The `int signo;` should be _before_ the opening `{`. This is an old-style function definition without a prototype. – Ian Abbott May 03 '17 at 13:44
  • @IanAbbott You are correct! While copying I made a mistake (I tried to put it through `gcc` and modified it to remove errors) – Nullpointer May 03 '17 at 13:46
  • @Nullpointer it has `extern` in front of it because otherwise that would be a function call, not a function declaration. – kfx May 03 '17 at 13:47
  • 1
    And `sbrk()` has the return type specified instead of `extern` because the return type of that one is not `int`. Writing `extern char *sbrk()` would give the same effect. – kfx May 03 '17 at 13:48
  • @kfx Thanks for the explanation. I have also edited my question to add that this code compiles correctly in Mac but gives error on Ubuntu. Why is this happening? Lastly, I don't think that the book is useless. It is an amazing book if you really want to learn about Unix concepts! – Nullpointer May 03 '17 at 13:49
  • 1
    The program is invalid according to the current C standard. It was OK back when (30 years ago). Perhaps the Mac OS people need to catch up with the program, or something. If you really want have a go at the corpse of pre-ANSI C, your best bet is probably the *first edition* of K&R. – n. m. could be an AI May 03 '17 at 13:55

2 Answers2

5
char *sbrk();
extern catcher();

These are function declarations, not function calls. The code you are reading is old style (pre-ANSI), and in subsequent (c99 or newer) C standards they are no longer valid.

You should add an explicit return type to the declaration of catcher(). The current implicit declaration means it has an int return type. However, the correct signature for a signal handler specifies no return value. When we add an explicit return type, the extern keyword is no longer needed and it can be removed.

sbrk is actually declared in a regular header, so remove the declaration and #include <unistd.h>. However, sbrk is BSD (and part of SUSv2) and not a standard C function, so you need to activate the declaration with #define _BSD_SOURCE or #define _XOPEN_SOURCE=500 before you include unistd.h.

Printf is declared in stdio.h, so let's include it. %u is used to print unsigned int. Pointers should be printed with the %p format specifier.

So, after some modernisation of the code:

#define _BSD_SOURCE
#include <stdio.h>
#include <signal.h>
#include <unistd.h>

void catcher();

char *cp;
int callno;

int main(void) {
    signal(SIGSEGV, catcher);
    cp = sbrk(O);  // You sure this should be an O and not a 0?
    printf("original brk value %u\n", cp);
    for (;;)
        *cp++ = 1; 
}


void catcher(int signo) {
    callno++;
    printf("caught sig %d %dth call at addr %p\n", signo, callno, cp);
    sbrk(256);
    signal(SIGSEGV, catcher); 
}

Please note that you should avoid calling printf from within a signal handler. See for example How to avoid using printf in a signal handler or Signal Handlers and Async-Signal Safety

Community
  • 1
  • 1
Klas Lindbäck
  • 33,105
  • 5
  • 57
  • 82
0

In addition to @Klas Lindbäck's answer, there are other problems with this code under the current C and POSIX standards:

#include < signal.h>
char *cp;
int callno;

main() {
    char *sbrk();
    extern catcher();

    signal(SIGSEGV, catcher);
    cp = sbrk(O);
    printf("original brk value %u\n", cp);
    for (;;)
    *cp++ = 1; 
}


catcher(signo) {
    int signo;
    callno++;
    printf("caught sig %d %dth call at addr %u\n", signo, callno, cp);
    sbrk(256);
    signal(SIGSEGV, catcher); 
}

Don't use sbrk()

sbrk() has been removed from the POSIX standard. It's no longer safe to use directly. From the Linux man page:

Avoid using brk() and sbrk(): the malloc(3) memory allocation package is the portable and comfortable way of allocating memory.

Why? Because numerous C library functions often use malloc()/calloc()/realloc()/... internally, and mixing malloc() and brk()/sbrk() calls in the same process is unsafe. malloc() implementations often use brk()/sbrk() internally, so using sbrk() in your own code might very well corrupt your heap.

Don't use signal()

signal() has significant issues. It's not consistent at all. Use sigaction() instead.

Community
  • 1
  • 1
Andrew Henle
  • 32,625
  • 3
  • 24
  • 56