Hello,
there is a lot (my attached smatch script found 234 occurences) of code
like
if (this)
HeapFree(GetProcessHeap(), 0, this);
HeapFree(GetProcessHeap(), 0, that);
which is redundant because after Win2000 (if i can trust the comment in
RtlFreeHeap) freeing a NULL pointer isn't an error. I've checked it
Win2000 and can confirm that.
I've also checked Win95 and Win98 and both don't crash when HeapFree'ing
a NULL but they return an error and SetLastError to 212 "ERROR_LOCKED".
In the above code the return code of HeapFree isn't checked so this
wouldn't matter if somebody would run the Wine code on that Win
versions.
So i propose to shorten that code to
HeapFree(GetProcessHeap(), 0, this);
HeapFree(GetProcessHeap(), 0, that);
I've done this already for the wininet dll (see patch to wine-patches)
and if it gets accepted i'll take care of the rest.
Advantages:
- less (redundant) code
- more pleasant to the eye
- for the wininet.dll it saves 1.1kB (FC2, gcc-3.3.3-7) in the generated
code (ok, the wininet.dll.so is 1.6 MB but still)
Disadvantages:
- if the pointer is NULL it takes longer for that code path. But i don't
think it matters cause this happens mostly only on the (unlikely)
error path.
bye
michael
--
Michael Stefaniuc Tel.: +49-711-96437-199
System Administration Fax.: +49-711-96437-111
Red Hat GmbH Email: [EMAIL PROTECTED]
Hauptstaetterstr. 58 http://www.redhat.de/
D-70178 Stuttgart
#!/usr/bin/perl
#
# This software may be used and distributed according to the terms
# of the GNU General Public License, incorporated herein by reference.
#
# Copyright 2004 Michael Stefaniuc
use smatch;
use strict;
sub error_msg() {
my $msg = shift;
print get_filename(), " ", get_lineno(), " ", get_cur_func(),
": HeapFree NULL check\n";
}
while(my $data = get_data()){
if ($data =~ /^if_cond var_decl\((.*)\)/) {
my $variable = $1;
$data = get_data();
if ($data =~ /^cmpstmt_start$/) {
$data = get_data();
}
if ($data =~ /^expr_stmt call_expr\(\(addr_expr
function_decl\(HeapFree\)\).*var_decl\($variable\)\)\)/) {
error_msg();
} else {
redo;
}
}
}
pgpp6cALBSsRX.pgp
Description: PGP signature
