On 10/19/06, Fedotov, Alexei A <[EMAIL PROTECTED]> 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
