0
\$\begingroup\$

I am writing a function that reads all consecutive blanks in a string, moves the pointer to the next non-blank char, and return a struct "token" that contains the blanks it found.

The main function is defined as

int match_ws(const char** p, struct token* tok)

where p points to the beginning of the strings to analyze, and tok will contain the result after the function returns. Return value is either 1 (successful), or 0.

[EDIT]

match_ws starts analyzing the string given as first input, and reads all consecutive whitespaces, while advancing the pointer too. It stores all blanks it found in the struct (second input) and returns 1. If it didn't find any blank, or too many, returns 0 without modifying tok.

So, if the input is const char* p = " . +" then after match_ws ran the tok structure will contain lexeme = " " (the four spaces at the beginning of p, and type = WS (enum type for "whitespace"). If the input is const char* p = ". ; )" then the function just returns 0 without modifying tok, because **p is not a whitespace (it's a dot).

Overall, this is part of a parser, and match_ws is the function that "consumes" all whitespaces -- not relevant for the parser. I believe the function behaves correctly, but since I am not experienced with the C language, I would like to have reviews about how I handled allocation of memory, pointer, pointer to pointers, function definition, etc.

[/EDIT]

Overall, my questions are about correct usage of pointers, pointers-to-pointers, good choice in the type of arguments (for the function definitions), and memory management.

I wrote my questions in the comments, but I am not so used to memory management and coding standards in C, so would appreciate a code review and all suggestions welcome.

I also added some boilerplate code so the file can be compiled and executed. As you can guess is part of a larger code, but I reduced it to be a minimal example.

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

#define MAX_LINE 100

/*
 * Some boilerplate code
*/
enum token_type {
    WS, // white spaces
    Eof
};

struct token{
    char* lexeme;
    enum token_type type;
};

char consume(const char** p);

/*
 * Main function to work on.
 * Questions:
 * - char ws[MAX_LINE] or using malloc?
 * - is it OK to initialize char ws with "" ?
 * - Am I appending char into ws in a optimal way?
 * - is it OK to return int and modify input argument tok, or should create a new tok and return it?
 * - makes sense to have pointer-to-pointer as input? Else what?
*/
int match_ws(const char** p, struct token* tok) {
    /*
     * Find all consecutives blanks starting from position p
     * Modify the input struct token.
     * Return 1 if execution was successful, 0 otherwise.
    */
    if ( **p != ' ' &&
         **p != '\t' &&
         **p != '\r' &&
         **p != '\n')
    {
        printf("Bad call to whitespace!!");
        return 0;
    }
    int i = 0;
    char c;
    char ws[MAX_LINE] = "";
    do {
        if (i >= MAX_LINE) {
            printf("Found more than %d white spaces", MAX_LINE);
            return 0;
        }
        c = consume(p);
        printf("WHITESPACE : Current char is |%c|\n", c);
        ws[i++] = c;
    }
    // Here p has been already advanced by the call to consume fn
    while (**p == ' ' || **p == '\t' || **p == '\r' || **p == '\n');
    tok->lexeme = ws;
    tok->type = WS;
    return 1;
}

/*
 * More boilerplate code to compile
*/
const char* type2char (enum token_type t) {
    switch (t)
    {
        case WS: return "WS";
        default: return "UNK";
    }
}

void print_token(struct token* p) {
    printf("<%s, %s>\n", p->lexeme, type2char(p->type));
}

char consume(const char** p) {
    // Advances the pointer while reading the input
    char c = **p;
    (*p)++;
    return c;
}

/*
 * Main.
 * Questions:
 * - is it OK to pass arguments to match_ws via '&' notation?
 * - Makes sense to have const char* line? Or just char* ?
 * - Should tok be initialized somehow?
 * - Is there anything that has to be free() eventually?
*/
int main() {

    const char* line = "     +";
    struct token tok;
    match_ws(&line, &tok);
    print_token(&tok);
}
\$\endgroup\$
  • 1
    \$\begingroup\$ I find it hard to understand the more exact outcome of match_ws interface to look deeper into the code. Possibly if you add some examples get a better grasp of the interface intention. \$\endgroup\$ – Bo R Feb 24 at 18:22
  • \$\begingroup\$ You seem to have registered with two different accounts; you'll need to flag for moderator attention to have your accounts merged. \$\endgroup\$ – Toby Speight Feb 25 at 12:06

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Browse other questions tagged or ask your own question.