On 16. 4. 2015 21:36, Florian Pritz wrote:
> On 13.04.2015 21:33, David Macek wrote:
>> + /* if we downloaded a DB, we want the .sig from the
>> same server */
>> + if(final_db_url != NULL) {
>> /* print final_db_url into a buffer (leave
>> space for .sig) */
>> len = strlen(final_db_url) + 5;
>> } else {
>> - /* print server + filename into a buffer (leave
>> space for .sig) */
>> + /* print server + filename into a buffer (leave
>> space for .db.sig) */
>> len = strlen(server) + strlen(db->treename) + 9;
>
> Comment changed, but not the code?
The comment was copied when the code was split into the two cases and the
parenthesized part apparently wasn't correctly updated.
> If you know where that + 9 comes from
> it might be a good idea to replace it with strlen("whatever") which the
> compile should optimize out later,
I didn't know gcc could do that, thanks for the tip.
> but writing it this way makes the
> whole thing a lot clearer.
First off, isn't this change better suited for a separate patch? If yes -- in
order to speed up the potential merge -- the comment change can be removed from
this patch and we can talk about the lengths in the context of another patch.
The 9 there is for "/", ".db.sig", and the null terminator. Would this be a
good way of re-writing the line?
strlen(server) + strlen("/") + strlen(db->treename) + strlen(".db.sig") + 1;
Warning: The following ideas might be too academic for this situation.
Since the strlen's are just duplicating the information contained in the format
string, wouldn't it be better to use snsprintf to count the buffer length? I
imagine the flow would be like this:
format = "...";
len = snprintf(NULL, 0, format, ...) + 1;
MALLOC(payload.fileurl, len, RET_ERR(handle, ALPM_ERR_MEMORY, -1));
snprintf(payload.fileurl, len, format, ...);
Only I don't know how to make it work nice with the conditional code. Consider
this:
format = final_db_url != NULL ? "%1$s.sig" : "%2$s/%3$s.db.sig";
len = snprintf(NULL, 0, format, final_db_url, server, db->treename) + 1;
MALLOC(payload.fileurl, len, RET_ERR(handle, ALPM_ERR_MEMORY, -1));
snprintf(payload.fileurl, len, format, final_db_url, server, db->treename);
This code is almost DRY, but various sources say that skipping parameters in
*printf is bad, so that doesn't work either.
Ideas?
--
David Macek
smime.p7s
Description: S/MIME Cryptographic Signature
