Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

addressed several warnings #64

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

addressed several warnings #64

wants to merge 1 commit into from

Conversation

Pp3ng
Copy link

@Pp3ng Pp3ng commented Oct 26, 2024

Pull Request Details

Summary

This pull request addresses several warnings in the file Tinyhttpd/httpd.c and improves the robustness of the code by handling return values of certain functions. Additionally, detailed comments have been added to the code to improve readability and maintainability. The changes include:

  1. Fixing the execl call to satisfy the non-null requirement.
  2. Handling the return value of fgets to ensure it is not ignored.
  3. Handling the return value of write to ensure it is not ignored.
  4. Adding detailed comments to the code.

Compilar Warnings

[I] ~/Tinyhttpd (master|✔) $ gcc -O3 -Wall httpd.c
httpd.c: In function ‘execute_cgi’:
httpd.c:282:9: warning: argument 2 null where non-null expected [-Wnonnull]
  282 |         execl(path, NULL);
      |         ^~~~~
In file included from httpd.c:20:
/usr/include/unistd.h:594:12: note: in a call to function ‘execl’ declared ‘nonnull’
  594 | extern int execl (const char *__path, const char *__arg, ...)
      |            ^~~~~
httpd.c:282:9: warning: not enough variable arguments to fit a sentinel [-Wformat=]
  282 |         execl(path, NULL);
      |         ^~~~~
httpd.c: In function ‘cat’:
httpd.c:167:5: warning: ignoring return value of ‘fgets’ declared with attribute ‘warn_unused_result’ [-Wunused-result]
  167 |     fgets(buf, sizeof(buf), resource);
      |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
httpd.c:171:9: warning: ignoring return value of ‘fgets’ declared with attribute ‘warn_unused_result’ [-Wunused-result]
  171 |         fgets(buf, sizeof(buf), resource);
      |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
httpd.c: In function ‘execute_cgi’:
httpd.c:290:17: warning: ignoring return value of ‘write’ declared with attribute ‘warn_unused_result’ [-Wunused-result]
  290 |                 write(cgi_input[1], &c, 1);
      |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~

Changes Made

  1. Fix execl Call:
  • Replaced NULL with an empty string "" in the execl call to satisfy the non-null requirement.
execl(path, path, (char *)NULL); // execute cgi script
  1. Handle fgets Return Value:
  • Added checks for the return value of fgets to ensure it is not ignored.
void cat(int client, FILE *resource)
{
    char buf[1024]; // buffer for reading file

    if (fgets(buf, sizeof(buf), resource) != NULL) { // read a line from the file
        while (!feof(resource))            // while not end of file
        {
            send(client, buf, strlen(buf), 0); // send the line to the client
            if (fgets(buf, sizeof(buf), resource) == NULL) { // read the next line from the file
                break;
            }
        }
    }
}
  1. Handle write Return Value:
  • Added checks for the return value of write to ensure it is not ignored.
if (write(cgi_input[1], &c, 1) < 0) {
    perror("write");
}
  1. Add Detailed Comments:
  • Added detailed comments to the code to improve readability and maintainability.

Code changes

void execute_cgi(int client, const char *path, const char *method, const char *query_string)
{
    char buf[1024];          // buffer for reading request
    int cgi_output[2];       // pipe for output
    int cgi_input[2];        // pipe for input
    pid_t pid;               // process id of child
    int status;              // status of child process
    int i;                   // index for iteration
    char c;                  // character buffer
    int numchars = 1;        // number of characters read
    int content_length = -1; // content length

    buf[0] = 'A';                                          // initialize buffer with 'A'(arbitrary)
    buf[1] = '\0';                                         // null-terminate buffer
    if (strcasecmp(method, "GET") == 0)                    // GET
        while ((numchars > 0) && strcmp("\n", buf))        // read & discard headers
            numchars = get_line(client, buf, sizeof(buf)); // read headers
    else if (strcasecmp(method, "POST") == 0)              // POST
    {
        numchars = get_line(client, buf, sizeof(buf));
        while ((numchars > 0) && strcmp("\n", buf)) // read headers
        {
            buf[15] = '\0';                                // null-terminate buffer
            if (strcasecmp(buf, "Content-Length:") == 0)   // if header is Content-Length
                content_length = atoi(&(buf[16]));         // get content length
            numchars = get_line(client, buf, sizeof(buf)); // read headers
        }
        if (content_length == -1) // if content length not found
        {
            bad_request(client);
            return;
        }
    }
    else /*HEAD or other*/
    {
        // do nothing
        return;
    }

    if (pipe(cgi_output) < 0) // check if creating pipe for output failed
    {
        cannot_execute(client);
        return;
    }
    if (pipe(cgi_input) < 0) // check if creating pipe for input failed
    {
        cannot_execute(client);
        return;
    }

    if ((pid = fork()) < 0) // check if forking failed
    {
        cannot_execute(client);
        return;
    }
    sprintf(buf, "HTTP/1.0 200 OK\r\n"); // send 200 OK response
    send(client, buf, strlen(buf), 0);
    if (pid == 0) /* child: CGI script */
    {
        char meth_env[255];   // environment variable for request method
        char query_env[255];  // environment variable for query string
        char length_env[255]; // environment variable for content length

        dup2(cgi_output[1], STDOUT);                    // redirect stdout to cgi_output
        dup2(cgi_input[0], STDIN);                      // redirect stdin to cgi_input
        close(cgi_output[0]);                           // close unused read end of output
        close(cgi_input[1]);                            // close unused write end of input
        sprintf(meth_env, "REQUEST_METHOD=%s", method); // set request method
        putenv(meth_env);                               // set environment variable
        if (strcasecmp(method, "GET") == 0)             /*GET */
        {
            sprintf(query_env, "QUERY_STRING=%s", query_string); // set query string
            putenv(query_env);                                   // set environment variable
        }
        else
        {                                                             /* POST */
            sprintf(length_env, "CONTENT_LENGTH=%d", content_length); // set content length
            putenv(length_env);                                       // set environment variable
        }
        execl(path, path, (char *)NULL); // execute cgi script
        exit(0);
    }
    else
    {                                            /* parent */
        close(cgi_output[1]);                    // close unused write end of output
        close(cgi_input[0]);                     // close unused read end of input
        if (strcasecmp(method, "POST") == 0)     // if method is POST
            for (i = 0; i < content_length; i++) // read content length bytes
            {
                recv(client, &c, 1, 0);
                if (write(cgi_input[1], &c, 1) < 0) {
                    perror("write");
                }
            }
        while (read(cgi_output[0], &c, 1) > 0) // read from cgi_output
            send(client, &c, 1, 0);            // send to client

        close(cgi_output[0]);
        close(cgi_input[1]);
        waitpid(pid, &status, 0); // wait for child process to finish
    }
}

void cat(int client, FILE *resource)
{
    char buf[1024]; // buffer for reading file

    if (fgets(buf, sizeof(buf), resource) != NULL) { // read a line from the file
        while (!feof(resource))            // while not end of file
        {
            send(client, buf, strlen(buf), 0); // send the line to the client
            if (fgets(buf, sizeof(buf), resource) == NULL) { // read the next line from the file
                break;
            }
        }
    }
}

void headers(int client, const char *filename)
{
    char buf[1024];
    (void)filename; /* could use filename to determine file type */

    strcpy(buf, "HTTP/1.0 200 OK\r\n");
    send(client, buf, strlen(buf), 0);
    strcpy(buf, SERVER_STRING);
    send(client, buf, strlen(buf), 0);
    sprintf(buf, "Content-Type: text/html\r\n");
    send(client, buf, strlen(buf), 0);
    strcpy(buf, "\r\n");
    send(client, buf, strlen(buf), 0);
}

void not_found(int client)
{
    char buf[1024];
    // standard response for not found
    sprintf(buf, "HTTP/1.0 404 NOT FOUND\r\n");
    send(client, buf, strlen(buf), 0);
    sprintf(buf, SERVER_STRING);
    send(client, buf, strlen(buf), 0);
}

Testing

  • Compiled the code to ensure no warnings are present.
  • Tested the server to ensure it handles requests correctly and the changes do not introduce any new issues.

@Liusandian
Copy link

Liusandian commented Oct 26, 2024 via email

@Pp3ng Pp3ng closed this Oct 26, 2024
@Pp3ng Pp3ng deleted the Pp3ng branch October 26, 2024 09:12
@Pp3ng Pp3ng restored the Pp3ng branch October 26, 2024 09:12
@Pp3ng Pp3ng reopened this Oct 26, 2024
@Pp3ng
Copy link
Author

Pp3ng commented Oct 26, 2024

I'm sorry. I was just trying to fix a typo.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants