:: Re: [DNG] Systemd Shims
Top Page
Delete this message
Reply to this message
Author: Rainer Weikusat
Date:  
To: dng
Subject: Re: [DNG] Systemd Shims
Edward Bartolo <edbarx@???> writes:
> I reorganised the C source code into header files and C code files. I
> also tested the backend to make sure the reorganisation of the code
> did not impact its functionality. I also included several 'patches' as
> suggested and made sure strcat does behave properly.
>
> Needless to state the obvious this code is still in development which
> means it is not yet ready to be used. However, for development
> evaluation purposes it works.
>
> Here is my humble C code.


There's one thing in there I consider very problematic:

int exec(const char* cmd, char* out)
{
        const int buf_size = 128;


        FILE * pipe = popen(cmd, "r");
        char buffer[buf_size];
        while(!feof(pipe)) {
                if(fgets(buffer, buf_size, pipe) != NULL)
                {
                        if (out != NULL)
                                strcat(out, buffer);
                                else strcpy(out, buffer);
                }
        }


        return pclose(pipe);
}


This blindly copies an indefinite amount of data into a caller provided
buffer but the caller cannot possibly know how large the buffer has to
be. Two actual problems related to that:


int connectionConnect(char* essid) //argv[2]
{
        char* s = 0;
        char command[1024];


        int result = snprintf(command, 1024, "/sbin/ifup wlan0 -i "IFACES_PATH"/%s", essid);
        if (result >= 1024)
                        return 105;
        printf(command);


        int q = exec(command, s);
        printf(s);
        return q;
}



int disconnectActiveConnection()
{
        char* s = 0;


        char command[1024];
        strcpy(command, "/sbin/ifdown wlan0");
        int q = exec(command, s);
        printf(s);
        return q;
} 


This runs exec with null pointer as second argument. In case there is
actually any output, the exec function will try to copy the data to
that and this should end up terminating the program with a SIGSEGV.

IMHO, the exec function should return the output written by the command
in a dynamically allocated buffer. And 'exec' is an unfortunate name as
it closely resembles the execve system call (and 'exec' is commonly used
to request such an operation, eg, by the shell). Some code implementing
the suggestion.

NB: I've tested that the buffer management works. The function will
return a NULL pointer to indicate that some kind of 'system error'
occured. Success-but-no-output returns an empty string. The caller must
free the returned string if memory leaks are to be avoided. The command
exit status is ignored. This may be a worthwhile avenue for future
extensions.

NB^2: This is also a nice demonstration that stdio isn't always the
easiest way to handle I/O.

------------
#include <stdlib.h>
#include <stdio.h>

#define BUF_SIZE    1024


char *read_command(const char *cmd) 
{
    FILE *fp;
    char *output, *p, *e, *tmp;
    ssize_t nr;
    int fd;


    fp = popen(cmd, "r");
    if (!fp) {
    perror("popen");
    return NULL;
    }


    output = malloc(BUF_SIZE);
    if (!output) {
    perror("malloc");
    return NULL;
    }


    fd = fileno(fp);
    p = output;
    e = output + BUF_SIZE;
    while ((nr = read(fd, p, e - p)) > 0) {
    p += nr;

    
    if (p == e) {
        nr = (p - output) * 2;
        tmp = realloc(output, nr);
        if (!tmp) {
        perror("realloc");
        goto err;
        }


        e = tmp + nr;
        p = tmp + (p - output);
        output = tmp;
    }
    }


    if (nr < 0) {
    perror("read");
    goto err;
    }


    pclose(fp);


    *p = 0;
    return output;


err:
    pclose(fp);
    free(output);


    return NULL;
}


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


    output = read_command(argv[1]);
    fputs(output, stdout);
    return 0;
}