HiveBrain v1.2.0
Get Started
← Back to all entries
patterncMinor

Impromptu TCP sender/receiver

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
senderimpromptutcpreceiver

Problem

Every once in a while I need to get some random computer thing done; so I write something to do it for me. This is one of those. It needed to be able to send text to, and receive text from, a server - without blocking. I am asking for feedback with the premise of what kind of code this is: A one-use hack.

#include 
#include 
#include 
#include 
#include 
#include 
#include 
#include 

#define BUFLEN 1024

volatile static int run = 1;

void sendx(int sd) {
    int sent, i;
    char buffer[BUFLEN], ch;
    do {
        i = 0;
        do {
            ch = getc(stdin);
            buffer[i] = ch;
            i++;
        } while (ch != '\n' && i  0 && run);
    close(sd);
    nothing[0] = '\n';
    write(0, nothing, 1);
    fflush(stdin);
}

int main(int argc, char **argv) {
    int sd, pid;
    struct sockaddr_in addr;

    memset(&addr, 0, sizeof(struct sockaddr));

    assert(argc == 3);
    sd = socket(AF_INET, SOCK_STREAM, 0);
    addr.sin_family = AF_INET;
    assert(inet_pton(AF_INET, argv[1], &addr.sin_addr) > 0);
    addr.sin_port = htons(atoi(argv[2]));
    assert(sd > -1);
    assert(connect(sd, (struct sockaddr *)&addr, sizeof(addr)) > -1);
    pid = fork();
    assert(pid > -1);
    if (pid == 0)   sendx(sd);
    else        recvx(sd);
    run = 0;
    return 0;
}

Solution

Some of the logic is nicely encapsulated in functions sendx and recvx, but then main disappoints with its lump of code. It would be better to leave main in charge of parsing and validating the command line arguments, and extract the rest to functions with descriptive names.

In main the assert(argc == 3) should come before the first memset: no need to initialize memory if you might not use it.

In sendx, the variable i is used for two purposes: loop variable for reading input, and input size. If you extract the input reading to a function that returns the number of characters read, then in sendx you can store that in a variable with a descriptive name, and the logic of the main responsibility of this method will become clearer.

Context

StackExchange Code Review Q#94608, answer score: 4

Revisions (0)

No revisions yet.