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> 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: > Request review for issue: > > > Bug 864607 - Empty certificate search in TPS results in httpd.worker segmentation fault then server error. > > Bugzilla: > https://bugzilla.redhat.com/show_bug.cgi?id=864607 > > Attachment: > > https://bugzilla.redhat.com/attachment.cgi?id=630239&action=diff > > _______________________________________________ > Pki-devel mailing list > Pki-devel(a)redhat.com > https://www.redhat.com/mailman/listinfo/pki-devel _______________________________________________ Pki-devel mailing list Pki-devel(a)redhat.com https://www.redhat.com/mailman/listinfo/pki-devel