Opened 12 years ago

Closed 11 years ago

#284 closed defect (fixed)

Compiler warning in urs_ext.c

Reported by: ole Owned by: jakeman
Priority: lowest Milestone:
Component: Compilation and installation Version:
Severity: normal Keywords:
Cc:

Description

The C-code urs_ext.c is used to read mux files generated by URS. When compiled it warns of a possible uninitialised pointer in line 184: Memory for the variable mytgs is only allocated when numSrc > 1 and may thus be NIL otherwise.

Here's an email trail that captures the discussion.


That's the spirit Chris! Have no mercy.

A slightly less brutal error message could be achieved by checking that numSrc is positive when it is first read in and failing with a more helpful error message is numSrc is <1. Neither of the subroutines make sense if numSrc is <1.

In the calling routine which I assume one of you wrote (the PyObject?) the same problem occurs since numSrc is used in a couple of malloc statements there as well. Therefore the sanity check needs to be in the PyObject? "*read_mux2" if you want to add it in.

I also note that numSrc isn't used in the statement which calls "_read_mux2" (line 117). Something called "pyweights->nd" is used instead. An earlier check (line 77) appears to require this to be 1, which means the "numSrc" variable in "_read_mux2" is always 1 which is not always true (but does ensure it is never zero at least). Am I reading your code correctly?

David


From: Thomas Chris Sent: Thursday, 29 May 2008 8:43 AM To: Nielsen Ole; Burbidge David; 'John Jakeman' Subject: RE: Possible uninitialised pointer in urs_ext.c [SEC=UNCLASSIFIED]

Yes, if anyone tries to demultiplex a quantity of zero mux2 files then they deserve a segmentation violation.


From: Nielsen Ole Sent: Wednesday, 28 May 2008 6:12 PM To: Thomas Chris; Burbidge David; 'John Jakeman' Subject: RE: Possible uninitialised pointer in urs_ext.c [SEC=UNCLASSIFIED]

... and John, Chris assures me that numSrc will be greater than zero when we aggregate events from the hazard map.

Cheers Ole


Dr. Ole Nielsen | Senior Computational Scientist Natural Hazard Impacts Project | Tsunami Risk Modelling team leader Risk & Impact Analysis Group | E: Ole.Nielsen@… Geospatial and Earth Monitoring Division | P: +61 2 6249 9048 Geoscience Australia | F: +61 2 6249 9986



From: Thomas Chris Sent: Wednesday, 28 May 2008 5:44 To: Nielsen Ole; Burbidge David; 'John Jakeman' Cc: Sexton Jane; Gray Duncan; Dhu Trevor Subject: RE: Possible uninitialised pointer in urs_ext.c [SEC=UNCLASSIFIED]

Whew! I wish a software engineer wasn't looking at code I wrote.

The original demuxing code allowed for the possibility of demuxing one or more mux2 files, weighting them and adding the results. This might be used for example if you have 10 unit sources, all comprising the one simulated earthquake. The mux files associated with the propagation from each unit source can be weighted (by the amount of slip on each unit source) and added to produce the time series for the combined event.

The variable numSrc is the number of mux files that one is demuxing, weighting and adding together.

Therefore it makes no sense whatsoever for the code to continue if numSrc == 0.

The structure tgsrwg holds some information about the data in the mux file, including the time step and the total length of the time series. If numSrc == 1 then we only need allocated space for one variable of type tgsrwg to hold this information. If numSrc > 1 then the code works by reading and processing the first mux file (in which process mytgs0 will be used), and then by looping over the other mux files and one by one reading, demuxing, weighting and adding, each time using the variable mytgs. Thus only if numSrc > 1 do we need to allocate space for mytgs.

Cheers

Chris


From: Nielsen Ole Sent: Wednesday, 28 May 2008 4:38 PM To: Burbidge David; Thomas Chris; 'John Jakeman' Cc: Sexton Jane; Gray Duncan; Dhu Trevor Subject: Possible uninitialised pointer in urs_ext.c [SEC=UNCLASSIFIED]

Hi guys

This is the C-code John Jakeman is using for reading mux files. When we compile it I noticed a possible uninitialised pointer in line 184: Memory for the variable mytgs is only allocated when numSrc > 1 and may thus be NIL otherwise.

The question is whether it makes sense for the code to continue if numSrc == 0 but when I tried that tests were failing. I'd like to get a sense from you what the implications might be in this case. I can see that there are other if statements further down that ensure that things are done when numSrc>1 but it is not clear to me

1: Why it makes sense to continue when numSrc == 0 2: If there is a risk of using mytgs uninitialised.

Can you help us?

Cheers and thanks Ole

Change History (1)

comment:1 Changed 11 years ago by nariman

  • Resolution set to fixed
  • Status changed from new to closed
Note: See TracTickets for help on using tickets.