G'day again,
Thank you for your responses to the three build summaries I sent out
yesterday. I've updated my Subversion repositories.
Attached is a patch for im/trunk/src/process/im_analyze.cpp, which
gets rid of some of the remaining "misleading indentation" warnings.
The comments in the patch, as well as the paranoid "set freed
stack-based pointers to NULL", are somewhat jarring relative to the
general tone and flow of the overall code of the module. I would
expect that some, or perhaps most, of these comments and paranoid code
would not be included in the immediate future; however, they may be
considered at some point. I'm using some level of paranoia in my
personal code, because I am striving to stay very clean in using and
freeing memory slabs and all associated memory pointers, so that it is
clean when run under Valgrind.
Although I've tackled a significant slab of code, there are more
opportunities for this kind of rewrite in a few places, perhaps
mostly further down in the same function; e.g. (line numbers are
rough figures since they assume the attached patch has been applied):
- Lines 834-844;
- Lines 963-974;
And a couple of places where we can rely on free(3) to quietly
ignore NULL pointers:
- Lines 707-708 ("if (local_data_cx) free(local_data_cx);");
- Line 1113 ("if (holes_perim) free(holes_perim);").
cheers,
sur-behoffski
programmer, Grouse Software
@@ -717,37 +717,41 @@
double* cm20 = (double*)malloc(region_count*sizeof(double));
double* cm02 = (double*)malloc(region_count*sizeof(double));
double* cm11 = (double*)malloc(region_count*sizeof(double));
-
+
+ // iCalcMoment returns "processing" boolean flag; this flag is cleared
+ // if the calculation is cancelled/abandoned for some reason. Variable
+ // name "ret" vague and could be improved.
ret = iCalcMoment(cm20, 2, 0, image, data_cx, data_cy, region_count, counter);
+ if (ret)
+ ret = iCalcMoment(cm02, 0, 2, image, data_cx, data_cy, region_count, counter);
+ if (ret)
+ ret = iCalcMoment(cm11, 1, 1, image, data_cx, data_cy, region_count, counter);
+
+ // If processing was stopped, clean up and exit.
if (!ret)
{
- if (local_data_area) free(local_data_area);
- if (local_data_cx) free(local_data_cx);
- if (local_data_cy) free(local_data_cy);
- if (cm20) free(cm20); if (cm02) free(cm02); if (cm11) free(cm11);
+ // Note that "free(NULL)" is legal, and merely does nothing.
+ free(local_data_area);
+ free(local_data_cx);
+ free(local_data_cy);
+ free(cm20);
+ free(cm02);
+ free(cm11);
+
+ // In addition to freeing pointers, be extremely paranoid and
+ // overwrite them with NULL to reduce the chances of use-after-free
+ // bugs.
+ local_data_area = NULL;
+ local_data_cx = NULL;
+ local_data_cy = NULL;
+ cm20 = NULL;
+ cm02 = NULL;
+ cm11 = NULL;
+
+ // Terminate the progress counter for this operation.
imProcessCounterEnd(counter);
return 0;
}
- ret = iCalcMoment(cm02, 0, 2, image, data_cx, data_cy, region_count, counter);
- if (!ret)
- {
- if (local_data_area) free(local_data_area);
- if (local_data_cx) free(local_data_cx);
- if (local_data_cy) free(local_data_cy);
- if (cm20) free(cm20); if (cm02) free(cm02); if (cm11) free(cm11);
- imProcessCounterEnd(counter);
- return 0;
- }
- ret = iCalcMoment(cm11, 1, 1, image, data_cx, data_cy, region_count, counter);
- if (!ret)
- {
- if (local_data_area) free(local_data_area);
- if (local_data_cx) free(local_data_cx);
- if (local_data_cy) free(local_data_cy);
- if (cm20) free(cm20); if (cm02) free(cm02); if (cm11) free(cm11);
- imProcessCounterEnd(counter);
- return 0;
- }
double *local_major_slope = 0, *local_minor_slope = 0;
if (!major_slope)
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Iup-users mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/iup-users
