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

Efficiency with strcpy, strcat and malloc

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

Problem

I'm still learning C, but I'm trying to make sure I've got a decent grasp on working with "strings" and data structures.

If possible, I'd like a little input on how I'm handling this and see if

  • it could be done more efficiently



  • I'm setting myself up for unforeseen negative consequences



  • I'm using the functions appropriately



void test(char *username, char *password) {

    printf("Checking password for %s - pw: %s\n",username,password);
    char *query1 = "SELECT password FROM logins WHERE email = '";
    char *query2 = "' LIMIT 1";

    char *querystring = malloc(strlen(query1) + strlen(username) + strlen(query2) * sizeof(char));

    strncpy(querystring,query1,strlen(query1));
    strncat(querystring,username,strlen(username));
    strncat(querystring,query2,strlen(query2));

    printf("Query string: %s\n",querystring);

    mysql_query(mysql_con,querystring);
    MYSQL_RES *result = mysql_store_result(mysql_con);

    int num_fields = mysql_num_fields(result);
    int num_rows = mysql_num_rows(result);

    if (num_rows != 0) {

        MYSQL_ROW row;
        printf("Query returned %i results with %i fields\n",num_rows,num_fields);

        row = mysql_fetch_row(result);

        printf("Password returned: %s\n",row[0]);

        int comparison = strncmp(password, row[0], strlen(password));

            if (comparison == 0) {
                printf("Passwords match!\n");
            } else {
                printf("Passwords do NOT match!\n");
            }

            }
    else {
        printf("No such user... Password is invalid");
    }
        free(querystring);
}


Output of program:

Checking password for test@blah.com - pw: 5f4dcc3b5aa765d61d8327deb882cf99
Query string: SELECT password FROM logins WHERE email = 'test@blah.com' LIMIT 1
Query returned 1 results with 1 fields
Password returned: 5f4dcc3b5aa765d61d8327deb882cf99
Passwords match!

Solution

Fairly good C. Few notes:

-
sizeof(char) is by 1 definition. It doesn't hurt to multiply by it, but still totally unnecessary.

-
malloc return value better be tested against NULL.

-
Calling functions of strn (emphasis on n) family in this context is kinda paranoid: you've already allocated enough memory; no need for extra safety. In fact, I'd rather go for sprintf("%s%s%s", querystring, query1, username, query2).

-
int comparison is just noise. It's OK to directly test results of strcmp(). BTW, strncmp in this context looks paranoid again.

-
I am not familiar with mysql. Shouldn't you release row somehow?

Context

StackExchange Code Review Q#61513, answer score: 7

Revisions (0)

No revisions yet.