On 11/25/19 8:02 AM, Fabian Grünbichler wrote: >>> + >>> + while (line != NULL) { >>> + next_line = memchr(line, '\n', >>> remaining); >>> + if (next_line != NULL) { >>> + *next_line = '\0'; >> unnecessary, just use (next_line or end) - line as boundaries and do a >> strncmp below[0] > but I actually want to ensure I compare both token and the full line, no > matter which one is longer. > > e.g., if the file contains (for whatever reason!) > someuser@pve!token asd213 > someotheruser@pve!token asdas-asdas-dsadsa > > and I compare against > > someuser@pve!token asd213-dfsdasads-asdsdas-dasds > > I don't want to compare across the newline! while it should never be > able to produce a false match (we reject token values as needle that > contain a newline), it seems like a good safeguard to ensure we only > match one full line at a time.. > >>> + } >>> + /* we ensure above that both are >>> properly terminated */ >> you ain't if next_line was NULL, i.e., no \n at EOF, at least if you >> do not do your out-of-bound write to *end = 0 anymore ;-) >> >>> + if (strcmp(line, rh->token) == 0) { >> [0] ...here >> >> effectively I'd go with: >> >> int bytes_read = memdb_read(memdb, "priv/token.cfg", &tmp); >> size_t remaining = bytes_read > 0 ? bytes_read : 0; >> if (tmp != NULL && remaining >= tokenlen) { >> char *line = (char *) tmp; >> char *next_newline; >> char *const end = line + remaining; >> >> while (line != NULL) { >> next_newline = memchr(line, '\n', remaining); >> if (next_newline != NULL) { >> *next_newline = '\0'; > so in practice I'd add another check here that the current line > (next_newline - line) is as long as rh->token, and if it is not, proceed > directly to the next line. this might also speed it up, since we now > only do the full string/memory comparison against stored values where > the full tokenid has the same length as the one we are looking for. > > if next_newline == NULL, then we must be in the last loop, and remaining > should be exactly tokenlen, so we could check that in an else branch and > bail if that check fails. >
sounds OK. >> } >> >> if (memcmp(line, rh->token, tokenlen) == 0) { >> result = 0; // found >> break; >> } >> >> line = next_newline; >> if (line != NULL) { >> line += 1; >> remaining = end - line; >> if (remaining < tokenlen) { >> result = -ENOENT; >> break; >> } >> } >> } >> if (line == NULL) { >> result = -ENOENT; >> } >> g_free(tmp); >> } else { >> cfs_debug("token: token.cfg does not exist - ENOENT"); >> result = -ENOENT; >> } >> >> here, the actually search part could be moved out to a is_line_in_conf >> function, as it's completely agnostic to tokens itself.. > sounds like a good idea - but where to put it? actually create the msg > and put most of the code in status.c, like for the property getter? or > keep the token stuff inline, and just move the line-search to > cfs-utils.c? > I'd rather use cfs-utils.c or a completely new file, e.g., ipc-utils.c, where alsot the config property getter stuff could be moved to. If unsure just keep it inline for now. _______________________________________________ pve-devel mailing list pve-devel@pve.proxmox.com https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel