Re: [Pki-devel] Review Request

Portion of patch, to address cfu's issues: cfu, please review. https://bugzilla.redhat.com/attachment.cgi?id=632378&action=edit ----- Original Message ----- From: "Christina Fu" <cfu(a)redhat.com&gt; To: pki-devel(a)redhat.com Sent: Tuesday, October 23, 2012 9:53:33 AM Subject: Re: [Pki-devel] Review Request The following are my review comments: - I see you replaced the PL_strcat() calls in mod_tokendb.cpp. How about the other PL_strcat() calls in other files? Do they not present any issue? - The following RA::Debug() message and comment seems a bit confusing for me as it seems to declare that it's about to truncate and yet it's actually trying to resize: A::Debug( "safe_injection_strcat, about to truncate, resize injection buffer: ", "current len: %d expected_len %d data_len: %d cur_injection_size %d",current_len, expected_len, cat_data_len, *injection_size ); /* We are going to get truncated! How about say something like "...resizing to avoid truncation..." - safe_injection_strcat() not sure of the role of the variable "result" as it only gets set once at the end, but returned many times in the middle. Unclear if 1 is success or 0 is. There is no comment (yeah, could you please provide comment for parameters and return value?). It is especially confusing when check_injection_size() comment says "returns 0 on success, 1 on failure" and yet safe_injection_strcat() has "if (check_res == 1), return result", where result was init'd to be 0, So does that mean it is the opposite of the check_injection_size()? Another thing is the callers ignore the return values from safe_injection_strcat() anyway, so why return anything at all? - Why only call check_injection_size 2nd time if still not enough? Christina On 10/22/2012 02:54 PM, John Magne wrote: _______________________________________________ Pki-devel mailing list Pki-devel(a)redhat.com https://www.redhat.com/mailman/listinfo/pki-devel