Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pass ndimx = 7 to dat_shape #1

Merged
merged 1 commit into from
Apr 10, 2018
Merged

Conversation

grahambell
Copy link
Contributor

This module's tests fail when being built with a version of Starlink
including HDS-v5. Passing ndimx > DAT__MXDIM to dat_shape is giving a
segmentation fault because hds_select's datShape is writing beyond
the size of a temporary array allocated by the fortran interface's
DAT_SHAPE.

The HDS fortran interface for DAT_SHAPE allocates a temporary array "cdims"
of size DAT__MXDIM (7) but then passes the given ndimx value to datShape.
It in turn this allocates a temporary array of the requested size and then
tries to copy ndimx values from there into cdims.  If ndimx > 7 it will
attempt to write past the end of cdims, causing a segmentation fault.
Copy link
Member

@timj timj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a bit confused. I thought that the second argument to dat_shape was indicating the amount of space to allocate and that HDS would only fill in up to 7 because it couldn't take more dimensions (and I am guessing that DAT__MXDIM wasn't easily available in Perl so I just picked a large number). I wish I had written those perl APIs to return the answer rather than having return arguments.

@timj
Copy link
Member

timj commented Apr 10, 2018

Shall I move the repo to Starlink org?

@grahambell
Copy link
Contributor Author

I think HDS would only fill up to 7, but new code in hds_select copies up to the specified number of values from its temporary array. The problem is really the fortran interface which allocates a 7-entry array but then passes on the given size, i.e. potentially claiming that the array it's passing is more than 7 entries long. There are lots of functions in the fortran interface which do this kind of thing:

   hdsdim cdims[DAT__MXDIM];
   /* ... */
   datShape( locator_c, *ndimx, cdims, ndim, status );

(so they will fail if *ndimx > DAT__MXDIM and the underlying code trusts the given ndimx).

Moving it to the Starlink organization sounds a good idea. I would like to also make a new CPAN release so that we can easily install the module here. (I think the SCUBA-2 quick-look uses it.)

@timj
Copy link
Member

timj commented Apr 10, 2018

@grahambell I think you now have control of this PR.

@grahambell grahambell merged commit df363bb into Starlink:master Apr 10, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants