12

The following code comes from example abo3.c from Insecure Programming — see also Why cast extern puts to a function pointer (void(*)(char*))&puts?:

int main(int argv,char **argc) {
    extern system,puts; 
    void (*fn)(char*)=(void(*)(char*))&system; // <==
    char buf[256];
    fn=(void(*)(char*))&puts;
    strcpy(buf,argc[1]);
    fn(argc[2]);
    exit(1);
}

Specifically this line:

void (*fn)(char*)=(void(*)(char*))&system;

I think that void (*fn)(char*) sounds like a lambda, but I know that it's not. Then, maybe this is only a play with parentheses, where void *fn(char*) is a declaration of a function and this function is referencing system? But why does the (char*) parameter have no name? Is this permitted?

Community
  • 1
  • 1
gui_cc2015
  • 261
  • 2
  • 8
  • 4
    The line `extern system, puts;` worries me. It declares that there are two externally defined integers (because there is no explicit type specified). These declarations shadow the declarations in `` and ``. However, at link time, the functions will probably be used to satisfy the references. Note that the `extern` line is invalid in C99 and beyond – and was dubious at best in pre-standard or C89/C90 standard C. – Jonathan Leffler Sep 16 '15 at 17:03
  • 9
    Your declaration `int main(int argv, char **argc)` is not technically invalid, but is 100% unorthodox. The integer argument is usually called `argc` (argument count), and the `char` double-pointer is usually called `argv` (argument vector). If you're going to use non-standard names, please do not, for everybody else's sanity, use the names reversed! – Jonathan Leffler Sep 16 '15 at 17:07
  • 2
    Use `int (*fn)(char const *)=&system;`, it's better to avoid casts. – 4566976 Sep 16 '15 at 17:17
  • @4566976: The cast is necessary as `system()` is `int(*)(const char*)`. The `&` unnecessary, however. – alk Sep 16 '15 at 17:20
  • 2
    @alk The cast is not necessary if you have the correct function pointer. It's bad to cast a function to another. I know that `&` is unnecessary. – 4566976 Sep 16 '15 at 17:24
  • Removed `ansi` tag. Tag explicitly says not to use it. – crashmstr Sep 16 '15 at 17:27
  • 2
    @NightSkyCode Absolutely not. The questions have almost same title but the contents is completely different. Moreover closing as duplicate to a bad question (score -1) isn't a good thing. Closure as duplicate should point to where the good question & information is (that's why you shouldn't take the date into account when doing so; an older question can be worse than a newer one asking the same thing). – Bakuriu Sep 16 '15 at 18:05
  • Refer to [this answer](http://stackoverflow.com/a/840504/1928852) for a good explanation on function pointers. – Enzo Ferber Sep 16 '15 at 18:24

7 Answers7

8

It declares the variable fn as a function pointer (to a function that has one argument of type char * and does not return anything (void).

This variable is initialised with the address of system - see http://linux.die.net/man/3/system. As noted from this page this will require the cast as given

Olipro
  • 3,489
  • 19
  • 25
Ed Heal
  • 59,252
  • 17
  • 87
  • 127
  • 1
    Since `system()` returns `int` instead of `void`, is this UB? – Kevin Sep 16 '15 at 18:21
  • @Kevin - Why do you think it is undefined behavior? – Ed Heal Sep 16 '15 at 18:32
  • 1
    Because you cast the function into the wrong type. It is *not* a `(void(*)(char*))`. [The converse cast is illegal](http://blogs.msdn.com/b/oldnewthing/archive/2004/01/19/60162.aspx), at least. – Kevin Sep 16 '15 at 18:34
  • 1
    *address* of `system`, not value. – Olipro Sep 16 '15 at 18:34
  • Oops - missed the `&` - That is not required. But then again the code is crap – Ed Heal Sep 16 '15 at 18:37
  • In this case, the `&` is required, because `system` is declared as just `extern system`, which means `extern int system`. So it will try to dereference `system` and grab an integer if you miss the `&`. – Kevin Sep 16 '15 at 18:41
4

Then, maybe this is only a play with parentheses,where void *fn(char *) is a declaration of a function and this function is referencing system, i think

void (*fn)(char *) is not a function , it is a function pointer. You can refer here to learn about function pointers.. And () do matter here you can't ignore them .

But why the parameter (char*) have no name? this is permitted?

Yes ,it is allowed to do so . Its name is not that important but its type is.

ameyCU
  • 16,489
  • 2
  • 26
  • 41
  • `void *fn(char *)` does not define a function pointer. The function pointer definition is `void (*fn)(char *)`. – alk Sep 16 '15 at 17:17
  • @alk Actually it is what he asked in that line , he missed `()` which are present in question's code . I will make it more clear. – ameyCU Sep 16 '15 at 17:40
3

It is pure and simple pointer to function, which is assigned address to the system call

Severin Pappadeux
  • 18,636
  • 3
  • 38
  • 64
2

First you go to the trouble of declaring a function pointer to system.

Then you throw it all away by redefining it to point to puts.

Then it appears that you try to index an int, but you have reversed the usual naming convention for main, which is

int main (int argc, char **argv)
Weather Vane
  • 33,872
  • 7
  • 36
  • 56
2
void (*fn)(char*)=(void(*)(char*))&system;

That line takes the address of the mis-declared symbol system (should be int(const char*), not implicit int), casts it to the type of fn and initializes that new local variable.
The type is void(*)(char*) or pointer to function receiving a single char* and returning nothing.


Well, that code is all kinds of bad:

  1. The traditional naming of the first two arguments to main is reversed:

    int main(int argv,char **argc)
    
  2. Declaring the standard-library-functions system and puts using "implicit int" as ints. That's not even the wrong function-type, but a data-type!

    extern system,puts;
    
  3. Casting the symbols address to a function-pointer is nearly sane. Now if only it was the right type... Anyway, on a box with data-pointers and code-pointers being the same size, it probably doesn't loose any information.

    void (*fn)(char*)=(void(*)(char*))&system; // <==
    fn=(void(*)(char*))&puts;
    
  4. Checking whether argv [!] is at least 2 should really not be omitted here:

    strcpy(buf,argc[1]);
    
  5. Calling a function through a function-pointer of wrong type is UB. Don't do that. Also, checking whether argv[!] is at least 3 before this point is not optional. fn(argc[2]);

  6. Relying on implicit declaration for strcpy and exit is also really bad. Neither has a prototype consistent with that! (they don't return int)

Deduplicator
  • 44,692
  • 7
  • 66
  • 118
1

It is a pointer to Function. Let me show you if you try to make an application in C that will use text menus where instead of switch I will use pointer to Function:

#include <stdio.h>
#include<unistd.h>

void clearScreen( const int x );
int exitMenu( void );
int mainMenu( void );
int updateSystem( void );
int installVlcFromPpa( void );
int installVlcFromSource( void );
int uninstallVLC( void );
int chooseOption( const int min, const int max );
void showMenu( const char *question, const char **options, int (**actions)( void ), const int length );
int installVLC( void );
int meniuVLC( void );
void startMenu( void );

int main( void ){
    startMenu();
    return 0;
}

void clearScreen( const int x ){
    int i = 0;
    for( ; i < x ; i++ ){
        printf( "\n" );
    }
}

int exitMenu( void ) {
    clearScreen( 100 );
    printf( "Exiting... Goodbye\n" );
    sleep( 1 );
    return 0;
}

int mainMenu( void ){
    clearScreen( 100 );
    printf( "\t\t\tMain Manu\n" );
    return 0;
}

int updateSystem( void ) {
    clearScreen( 100 );
    printf( "System update...\n" );
    sleep( 1 );
    return 1;
}

int installVlcFromPpa( void ) {
    clearScreen( 100 );
    printf("Install VLC from PPA \n");
    sleep( 1 );
    return 0;
}

int installVlcFromSource( void ) {
    clearScreen( 100 );
    printf( "Install VLC from Source \n" );
    sleep( 1 );
    return 0;
}

int uninstallVLC( void ) {
    clearScreen( 100 );
    printf( "Uninstall VLC... \n" );
    sleep( 1 );
    return 1;
}

int chooseOption( const int min, const int max ){
    int option,check;
    char c;

    do{
        printf( "Choose an Option:\t" );

        if( scanf( "%d%c", &option, &c ) == 0 || c != '\n' ){
            while( ( check = getchar() ) != 0 && check != '\n' );
            printf( "\tThe option has to be between %d and %d\n\n", min, max );
        }else if( option < min || option > max ){
            printf( "\tThe option has to be between %d and %d\n\n", min, max );
        }else{
            break;
        }
    }while( 1 );

    return option;
}

void showMenu( const char *question, const char **options, int ( **actions )( void ), const int length) {
    int choose = 0;
    int repeat = 1;
    int i;
    int ( *act )( void );

    do {
        printf( "\n\t %s \n", question );

        for( i = 0 ; i < length ; i++ ) {
            printf( "%d. %s\n", (i+1), options[i] );
        }

        choose = chooseOption( 1,length );
        printf( " \n" );

        act = actions[ choose - 1 ];
        repeat = act();

        if( choose == 3 ){
            repeat = 0;
        }
    }while( repeat == 1 );
}

int installVLC( void ) {
    clearScreen( 100 );
    const char *question = "Installing VLC from:";
    const char *options[10] = { "PPA", "Source", "Back to VLC menu" };
    int ( *actions[] )( void ) = { installVlcFromPpa, installVlcFromSource, mainMenu };

    size_t len = sizeof(actions) / sizeof (actions[0]);
    showMenu( question, options, actions, (int)len );
    return 1;
}

int meniuVLC( void ) {
    clearScreen( 100 );
    const char *question = "VLC Options";
    const char *options[10] = { "Install VLC.", "Uninstall VLC.", "Back to Menu." };
    int ( *actions[] )( void ) = { installVLC, uninstallVLC, mainMenu };

    size_t len = sizeof(actions) / sizeof (actions[0]);
    showMenu( question, options, actions, (int)len );

    return 1;
}

void startMenu( void ){
    clearScreen( 100 );
    const char *question = "Choose a Menu:";
    const char *options[10] = { "Update system.", "Install VLC", "Quit" };
    int ( *actions[] )( void ) = { updateSystem, meniuVLC, exitMenu };

    size_t len = sizeof(actions) / sizeof (actions[0]);

    showMenu( question, options, actions, (int)len );
}

Compile it and try it.

Michi
  • 5,175
  • 7
  • 33
  • 58
1

The line is declaring a variable of a function pointer that takes a parameter and returns nothing (void), this is needed along with the typecast to the same function prototype as extern functions are similar in their type to a void* pointer, they are not early bound during compilation.

Nader
  • 47
  • 1
  • 8