harmony-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Andrew Zhang" <zhanghuang...@gmail.com>
Subject Re: HARMONY-1752: patch review
Date Thu, 19 Oct 2006 00:59:07 GMT
On 10/19/06, Fedotov, Alexei A <alexei.a.fedotov@intel.com> wrote:
>
> Hello Ilya,
>
>
>
> I really like your patch from
> http://issues.apache.org/jira/browse/HARMONY-1752. Let me participate in
> a way I'm able to.
>
>
>
> I cannot say why calling free(send_buf) when send_buf == NULL makes me
> feel a bit uncomfortable.


It's safe to free NULL pointer.

I also prefer a descriptive name for a label
> cleanup1 (eg cleanup_buf).
>
>
>
> I tried to track that guy who used cleanup labels in his C code.


Let the author speak for himself, but I think it's ok. It's a frequentely
used error handling sytle in C programming. There's a logical problem in
OSNetworkSystemWin32.c, the cleanup may free an invalid pointer fdset_read.
But I perfer to add NULL initiliazation when declaring fdset_read, say,
fd_set * fdset_read = NULL;

comments?

modules/luni/src/main/native/luni/linux/OSNetworkSystemLinux.c:
>
> The Linux version seems to contain exactly the same problem as you fix
> in modules/luni/src/main/native/luni/windows/OSNetworkSystemWin32.c
>
>
>
> modules/luni/src/main/native/luni/shared/luniglob.c
>
> cleanup:
>
>    if (props) {
>
>        properties_free(PORTLIB, props);
>
>    }
>
>    if (bootDirectory) {
>
>        hymem_free_memory(bootDirectory);
>
>    }
>
> The first "if" always fails if we come here using goto - we can put the
> label after the first if. I suggest replacing the second if with
> assert(!bootDirectory)
>
>
>
> modules/archive/src/main/native/archive/shared/jarfile.c:
>
> zip_freeZipEntry is not called on some paths - is it a memory leak?
>
>
>
> modules/luni/src/main/native/launcher/shared/main.c
>
> if (vm_args.options)
>
>    {
>
>      hymem_free_memory (vm_args.options);
>
>    }
>
> Should we put here assert(!vm_args.options)?
>
>
>
>
>
> With best regards,
> Alexei Fedotov,
> Intel Java & XML Engineering
>
>
>
>
>


-- 
Best regards,
Andrew Zhang

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message