[Pki-devel] Review Request

[John Magne]

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 at redhat.com>
To: pki-devel at 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 at redhat.com
> https://www.redhat.com/mailman/listinfo/pki-devel

_______________________________________________
Pki-devel mailing list
Pki-devel at redhat.com
https://www.redhat.com/mailman/listinfo/pki-devel