[webkit-reviews] review denied: [Bug 22541] This program is capable to store all the cookies in a hidden folder in ur home directory and it is also capable to render a SSL certificate on the terminal.. : [Attachment 25569] program is capable to store all the cookies in a hidden folder in ur home directory and it is also capable to render a SSL certificate on the terminal..

[bugzilla-daemon at webkit.org]

Oliver Hunt <oliver at apple.com> has denied Nimish Nayak <nimishnayk at gmail.com>'s
request for review:
Bug 22541: This program is capable to store all the cookies in a hidden folder
in ur home directory and it is also capable to render a SSL certificate on the
terminal..
https://bugs.webkit.org/show_bug.cgi?id=22541

Attachment 25569: program is capable to store all the cookies in a hidden
folder in ur home directory and it is also capable to render a SSL certificate
on the terminal..
https://bugs.webkit.org/attachment.cgi?id=25569&action=review

------- Additional Comments from Oliver Hunt <oliver at apple.com>
I'm not a gtk reviewer in general but there a few style issues we can clean up
prior to getting alp and co to have a look:



> + * Copyright (C) 2008 sleepy_cat.

This should be your actual name :D

>   * All rights reserved.
>   *
>   * Redistribution and use in source and binary forms, with or without
> @@ -29,7 +30,15 @@
>   * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
>   */
>  
> +#include "stdio.h"
> +#include "stdlib.h"
> +#include "unistd.h"
> +#include "string.h"
> +#include "curl/curl.h"
> +#include "curl/types.h"
> +#include "curl/easy.h"
These new #includes should go in alphabetical order with all the other
#includes further down in the file 

>	       curl_easy_getinfo(d->m_handle, CURLINFO_EFFECTIVE_URL, &url);
> -	       fprintf(stderr, "Curl ERROR for url='%s', error: '%s'\n", url,
curl_easy_strerror(msg->data.result));
> +	       printf("Curl ERROR for url='%s', error: '%s'\n", url,
curl_easy_strerror(msg->data.result));

This should probably still be fprintf rather than printf

> -	   if (base64Decode(data.latin1().data(), data.latin1().length(), out)
&& out.size() > 0)
> +	   if (base64Decode(data.latin1().data(), data.latin1().length(), out))

>	       client->didReceiveData(handle, out.data(), out.size(), 0);

> -	   if (data.length() > 0)
> -	       client->didReceiveData(handle, reinterpret_cast<const
char*>(data.characters()), data.length() * sizeof(UChar), 0);
> +	   client->didReceiveData(handle, reinterpret_cast<const
char*>(data.characters()), data.length() * sizeof(UChar), 0);
>      }

Why did you remove the out.size() checks?


>  
>      client->didFinishLoading(handle);
> @@ -582,11 +613,14 @@
>  
>      m_runningJobs++;
>      CURLMcode ret = curl_multi_add_handle(m_curlMultiHandle,
job->getInternal()->m_handle);
> +    
> +    
>      // don't call perform, because events must be async
>      // timeout will occur and do curl_multi_perform
> +   
>      if (ret && ret != CURLM_CALL_MULTI_PERFORM) {

Why add the additional newlines :D

>  #ifndef NDEBUG
> -	   fprintf(stderr, "Error %d starting job %s\n", ret,
encodeWithURLEscapeSequences(job->request().url().string()).latin1().data());
> +	   printf("Error %d starting job %s\n", ret,
encodeWithURLEscapeSequences(job->request().url().string()).latin1().data());
>  #endif

printf vs. fprintf again

> +    strcpy(path , loc);
> +strcat(path , "/.midori");

I don't believe strcpy and strcat are unicode safe so i'm 

> +	   if(!res && ci) {
> +	     int i;
> +	     printf("%d certs!\n", ci->num_of_certs);
This looks like you left unintentional debugging code :D