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: