Thanks, that ndiagonal_exclusions should definitely be initialised to 0
(the nexcluded_regions was set to 0 at the bottom of that if block in the
else statement). This could definitely throw some compilers, you should
definitely always initialise everything. So I've fixed that already.
On the more general point, I didn't know that there was an issue with the
typedef struct naming. *All* the structs in Analysis follow that
convention. The struct one is only ever (intended to be) used in memory
allocation (when of course you need to know how much memory to allocate),
whereas the pointer is used for all other purposes. The Linux compiler
(at least) seems to be pretty good if I accidentally slip on up the uses
of these pointers (by which I mean if I accidentally forget to use a *, or
whatever, it complains) but perhaps other compilers are not so bright. If
this is possibly going to be a problem then I better go through and rename
all the structs (I can put an _ in front or something).
I'd be amazed if this is causing your problem, since it's to do with peak
finding, which should not be called just when a project is being opened.
Wayne
On Thu, 26 May 2005, Bruce D. Ray wrote:
> >Thanks, that should help us hunt it down if it is in the C code by itself.
> >That doesn't change very much these days, although there was a bit of a
> >change due to diagonal and other exclusion options added in peak finding
> >(but that is peak finding, which doesn't sound like the problem here).
>
>
> Well, I did a diff on all code in /analysis1.02/ccpnmr/ccpnmr1.0/c/ccpnmr/analysis
> versus /analysis1.02/ccpnmr/ccpnmr1.0/c/ccpnmr/analysis
>
> I see a couple of things that bother me. Probably these are not related
> to the abort I got. Quite probably I'm being overly cautious as well,
> but they might merit some looking into anyway.
>
> First, in peak_list.h a new structure is defined at lines 64-74:
>
> > typedef struct Diagonal_exclusion
> > {
> > int dim1;
> > int dim2;
> > float a1;
> > float a2;
> > float b12;
> > float d;
> > } *Diagonal_exclusion;
> >
>
> I would think it would be bad practice to give the same name to both
> a structure and a type because it relies on the compiler too heavily
> for my tastes.
>
> Moving on, at about line 78 in py_peak_list.c I read:
>
> > static Status alloc_diagonal_memory(Diagonal_exclusion *diagonal_exclusion, int nexclusions)
>
> Now the typedef name is a pointer, and I hope the typedef name
> is used here and not the struct name. Thus, the first parameter
> is supposed to be a pointer to a pointer, and the routine stores
> a pointer to allocated memory at whatever the first parameter points
> to. The only call to this allocation routine does treat this
> properly, but only if every single c compiler chooses the correct
> definition of Diagonal_exclusion. I believe c compilers are supposed
> to choose the typedef name over the struct name, but I swore off
> relying on what a compiler is supposed to do back in 1967 when I
> got bitten by what CDC3600 compiler actually did instead of what the
> documentation claimed it was supposed to do.
>
>
> Secondly, I see a number of places where ndiagonal_exclusions is
> tested to see if it is 0, but the only place I can find where
> ndiagonal_exclusions is ever set is inside of a block that begins
> at about line 385 in py_peak_list.c as:
>
> > if (diagonal_exclusion_obj)
>
> As I read this, ndiagonal_exclusions only has a defined value if
> diagonal_exclusion_obj is true. I believe actively setting
> ndiagonal_exclusions to 0 when diagonal_exclusion_obj is false
> might be a good idea. Undoubtedly there is some place in one
> of the other code directories I haven't checked where
> ndiagonal_exclusions is given a defined value, and I'm being
> overly cautious.
>
> I see the same problem for nexcluded_regions as well. Again,
> nexcluded_regions is tested to see if it is 0 several places,
> but is only set in a block between lines 382 and 510 in
> py_peak_list.c that begins:
>
> > if (excluded_region_obj)
>
> As I read this, if excluded_region_obj is false, nexcluded_regions
> may not have a defined value. Here again, I believe actively
> setting nexcluded_regions to 0 when excluded_region_obj is false
> might be a good idea. Again, without a doubt there is some place
> in one of the other code directories I haven't checked where
> ndiagonal_exclusions is given a defined value, and I'm being
> overly cautious.
>
>
> These are the items that stood out to me with just a quick glance
> at the diff. Probably none of this is related to the problem I
> reported.
>
>
> Sincerely,
>
>
> --
> Bruce D. Ray, Ph.D.
> Associate Scientist, and Operations Director
> NMR Center
> IUPUI
> Physics Dept.
> 402 N. Blackford St.
> Indianapolis, IN 46202-3273
>
|