Nicholas James

  • More
  • CR-8202
  • summarized and closed
This fix was incorporated into OPENAM-6859, so I'm closing this review.

This fix was incorporated into OPENAM-6859, so I'm closing this review.

ok thats what i thought. i'm done here.

ok thats what i thought. i'm done here.

Can i just check i understand the strategy here - is it that since either the timer thread or the waiting thread can finish after the other, we are preventing the second one from trying to use a mu...

Can i just check i understand the strategy here - is it that since either the timer thread or the waiting thread can finish after the other, we are preventing the second one from trying to use a mutex/condition when it has already been destroyed?

should this be *e = NULL;

should this be
*e = NULL;

Can we set the hostmap_sz after the malloc has succeeded, so that it is 0 if the malloc fails?

Can we set the hostmap_sz after the malloc has succeeded, so that it is 0 if the malloc fails?

i have no preference the code might be ok as is: the first one is strdup'ed and subsequently realloced.

i have no preference
the code might be ok as is: the first one is strdup'ed and subsequently realloced.

sure. good spot. or strcpy?

sure. good spot. or strcpy?

ok

ok

should these header values be concatenated without spaces between them?

should these header values be concatenated without spaces between them?

OK by me. Its the trusted ca certs that have ben a problem i guess.

OK by me. Its the trusted ca certs that have ben a problem i guess.

i've been wondering whether it would be better to blow up if other bits are missing, in case openssl expects that too: like the client cert file. i don't know...

i've been wondering whether it would be better to blow up if other bits are missing, in case openssl expects that too: like the client cert file. i don't know...

i had here ..} else if ca_file_loaded { set verify peer, set verify depth } else { the above error }

i had here

..} else if ca_file_loaded { set verify peer, set verify depth } else { the above error } 
can we update this to cover failing if verifying and trusted ca file not specified (as well as not loaded)?

can we update this to cover failing if verifying and trusted ca file not specified (as well as not loaded)?

Is this nginx code or https://github.com/joyent/http-parser/commit/8dabce6ec7142319bc5c883ff53bf7302f0d83ce
I think I need to cause this error if (after line 638) the ca file is not supplied and verifypeer is true.

I think I need to cause this error if (after line 638) the ca file is not supplied and verifypeer is true.

Agent 4 should fail when it is set to verify peer and the trusted ca certificates file cannot be loaded
Agent 4 should fail when it is set to verify peer and the trusted ca certificates file cannot be loaded
  • More
  • CR-8150
  • summarized and closed
  • More
  • CR-8185
  • summarized and closed
  • More
  • CR-8193
  • summarized and closed
e->value is already checked, so NOTNULL is not used here.

e->value is already checked, so NOTNULL is not used here.

no problems with this one.

no problems with this one.

this might be an accident

this might be an accident

Change property file line endings on non-windows systems.
Change property file line endings on non-windows systems.
Okay, will check in. Thanks

Okay, will check in. Thanks

Ah you mean if the string to be copied is shorted than n, will it set from strlen(s) to n as '\0'? It will work ok as it is won't it?

Ah you mean if the string to be copied is shorted than n, will it set from strlen(s) to n as '\0'?
It will work ok as it is won't it?