How can I make this better?#include <stdio.h>#include <sys/types.h>#include <sys/stat.h>#include <fcntl.h>#include <unistd.h>#include <string.h>#include <stdlib.h>#define ARP_CACHE       /proc/net/arp#define ARP_BUFFER_LEN  1024#define ARP_DELIM        int readLine(int Fd, char *Buffer){    if (Fd < 0)    {        return -1;    }    char ch;    size_t Read = 0;    while (read(Fd, (Buffer + Read), 1))    {        ch = Buffer[Read];        if (ch == '\n')        {            break;        }        Read++;    }    if (Read)    {        Buffer[Read] = 0;        return 0;    }    return -1;}char * getField(char * Line_Arg, int Field){    char * Line = malloc(strlen(Line_Arg)), *ptr;    memcpy(Line, Line_Arg, strlen(Line_Arg));    ptr = Line;    char * s;     s = strtok(Line, ARP_DELIM);    while (Field && s)    {        s = strtok(NULL, ARP_DELIM);        Field--;    };    char * ret;    if (s)    {        ret = (char*)malloc(strlen(s) + 1);        memset(ret, 0, strlen(s) + 1);        memcpy(ret, s, strlen(s));    }    free(ptr);    return s ? ret : NULL;}int main(){    int Fd = open(ARP_CACHE, O_RDONLY);    if (Fd < 0)    {        fprintf(stdout, Arp Cache: Failed to open file \%s\\n, ARP_CACHE);        return 1;    }    char Buffer[ARP_BUFFER_LEN];    /* Ignore first line */    int Ret = readLine(Fd, &Buffer[0]);    Ret = readLine(Fd, &Buffer[0]);    int count = 0;    while (Ret == 0)    {        char * Line;        Line = &Buffer[0];        /* Get Ip, Mac, Interface */         char * Ip       = getField(Line, 0);        char * Mac      = getField(Line, 3);        char * IfaceStr = getField(Line, 5);        fprintf(stdout, %03d: Mac Address of [%s] on [%s] is \%s\\n,                ++count, Ip, IfaceStr, Mac);        free(Ip);        free(Mac);        free(IfaceStr);        Ret = readLine(Fd, &Buffer[0]);    }    close(Fd);    return Ret;}
Parsing ARP cache in C
c;parsing;linux;networking
ConceptI don't believe that there is a cross-platform way to read the ARP cache.  From the use of /proc filesystem, I deduce that you are targeting Linux.  In Linux, you could read /proc/net/arp as you have done, or run the command ip neigh, which does something similar to your program.  (On OS X, you could run arp -a -n instead.)A comment worth a thousand wordsYou should include a comment like this in your program:/** * /proc/net/arp looks like this: * * IP address       HW type     Flags       HW address            Mask     Device * 192.168.12.31    0x1         0x2         00:09:6b:00:02:03     *        eth0 * 192.168.12.70    0x1         0x2         00:01:02:38:4c:85     *        eth0 */That picture tells me everything I need to know about what you are trying to accomplish  you need hardly explain further.  Conversely, not having that picture makes me work a lot harder to reverse-engineer your code.Minutiae&Buffer[0] is usually written as Buffer.You repeat the readLine() call  once before entering the loop, once at the end of the loop.  This is a good opportunity to make use of C's support for side-effects.while (0 == (Ret = readLine(Fd, Buffer))) {    }Since ARP_CACHE is a compile-time constant string, you can just writefprintf(stdout, Arp Cache: Failed to open file \ ARP_CACHE \\n);instead of doing a %s substitution at runtime.Reading one byte at a time is wasteful.Print errors to standard error; don't contaminate standard output.I suggest naming your variables to be consistent with the /proc/net/arp header fields.  For example, device instead of IfaceStr.  Also, be consistent with capitalization: count starts with lowercase, which is more common.Big-picture issuesThe function int readLine(int Fd, char *Buffer) is kind of susceptible to buffer overflow.  I can deduce that fact from the function signature: you pass a pointer to a buffer without also passing the size, so it seems unlikely that readLine would know how to stop when the buffer fills up.  You could hard-code it to use ARP_BUFFER_LEN as a limit, but it would be unfortunate to cripple an otherwise generic function like that.  It would be better to pass in the buffer size explicitly.  That's a general pattern you'll see in C APIs: a pointer to a buffer is frequently accompanied by the buffer's size.In practice, /proc/net/arp should never contain a line long enough to overflow a 1024-byte buffer, so you're safe.  Still, you should follow idiomatic C conventions.Once you modify readLine() to take a size parameter, you'll find that you've just reinvented fgets().In C, passing strings from a function to its caller is usually avoided, since it involves malloc(), which introduces the potential for memory leaks.  Rather, have the caller pass a buffer and size, like how fgets() and scanf() work.The only good reason to return a string that was allocated using malloc() would be to support arbitrary-length results.  At first glance, your getField() accomplishes that, as it allocates a buffer as large as strlen(Line_Arg).  (You forgot to add a byte to accommodate the terminating NUL character, by the way.)  But, that turns out not to be the case, since Line_Arg itself is not a string of arbitrary length.  It's either less than ARP_BUFFER_LEN bytes long (if you got lucky) or a buffer overflow (as discussed in (1) above).You seem to be working very hard at I/O and string processing.  Why not just use fscanf()?The whole program, then, can be simplified to#include <stdio.h>/** * Macros to turn a numeric macro into a string literal.  See * https://gcc.gnu.org/onlinedocs/cpp/Stringification.html */#define xstr(s) str(s)#define str(s) #s#define ARP_CACHE       /proc/net/arp#define ARP_STRING_LEN  1023#define ARP_BUFFER_LEN  (ARP_STRING_LEN + 1)/* Format for fscanf() to read the 1st, 4th, and 6th space-delimited fields */#define ARP_LINE_FORMAT % xstr(ARP_STRING_LEN) s %*s %*s  \                        % xstr(ARP_STRING_LEN) s %*s  \                        % xstr(ARP_STRING_LEN) sint main(){    FILE *arpCache = fopen(ARP_CACHE, r);    if (!arpCache)    {        perror(Arp Cache: Failed to open file \ ARP_CACHE \);        return 1;    }    /* Ignore the first line, which contains the header */    char header[ARP_BUFFER_LEN];    if (!fgets(header, sizeof(header), arpCache))    {        return 1;    }    char ipAddr[ARP_BUFFER_LEN], hwAddr[ARP_BUFFER_LEN], device[ARP_BUFFER_LEN];    int count = 0;    while (3 == fscanf(arpCache, ARP_LINE_FORMAT, ipAddr, hwAddr, device))    {        printf(%03d: Mac Address of [%s] on [%s] is \%s\\n,                ++count, ipAddr, device, hwAddr);    }    fclose(arpCache);    return 0;}