patterncMinor
Efficiency with strcpy, strcat and malloc
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
Output of program:
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:
-
-
-
Calling functions of
-
-
I am not familiar with mysql. Shouldn't you release
-
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.