Skip to content

Hdf5 bugfix#71

Merged
MarkRivers merged 3 commits intoareaDetector:hdf5-bugfixfrom
aglowacki:hdf5-bugfix
Mar 4, 2015
Merged

Hdf5 bugfix#71
MarkRivers merged 3 commits intoareaDetector:hdf5-bugfixfrom
aglowacki:hdf5-bugfix

Conversation

@aglowacki
Copy link
Copy Markdown

This update fixes the crash for issue #66 “ndattr_default=true” is not defined. The current logic is that if it is not defined it will not save them anywhere.

This partly fixes issue #65. It should now save attributes under constant datasets. It does not save attributes under ndattribute datasets.

MarkRivers added a commit that referenced this pull request Mar 4, 2015
@MarkRivers MarkRivers merged commit cf708b2 into areaDetector:hdf5-bugfix Mar 4, 2015
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

With writeHdfConstDatasets() commented out at this point, this function is not used anywhere else. Is it no longer required and can be deleted? It should not be left in the code like this if it is not used...

I must admit I've lost track a bit in these many changes so I don't have the overview to determine if this function can go...

@MarkRivers
Copy link
Copy Markdown
Member

I just tested this fix. There were several problems:

  • It does not compile on Linux, and had a compiler warning due to no return value from function. Please test that code compiles without error or warning on both Linux and Windows. I have fixed the problems and pushed to github.
  • When testing with test2_attributes.xml and test2_layout_no_ndattr_default.xml there are HDF5 error messages:
    HDF5-DIAG: Error detected in HDF5 (1.8.7) thread 0:
    #000: H5G.c line 734 in H5Gclose(): not a group
    major: Invalid arguments to routine
    minor: Inappropriate type
    When testing with test2_layout.xml there are no errors, so this must be related to the lack of the nd_attr_default tag. So it no longer crashes, but it does produce HDF5 error messages.

I verified that attributes of constant datasets are now stored correctly.

@aglowacki
Copy link
Copy Markdown
Author

That is strange because I compiled on linux and didn't get any errors.

The writeHdfConstDatasets() function is not needed anymore, I forgot to remove it.

@MarkRivers
Copy link
Copy Markdown
Member

What version of asyn are you using on Linux? In asyn R4-21 I added this:


Changed the asynTrace print, printIO, vprint, vprintIO functions so they use EPICS_PRINTF_STYLE. This causes the GCC (version 3.0 and higher) and clang compilers to check the agreement of format strings with function arguments when using asynPrint() and asynPrintIO(), just as they do with printf(). This is very helpful in finding errors, and uncovered a number in asyn itself, which have been fixed.


The error and fix were the following:
asynPrint(this->pasynUserSelf, ASYN_TRACE_ERROR, "%s::writeHdfAttributes unknown type: unable to create attribute: %s\n",

  •          driverName, attr.get_name());
    
  •          driverName, attr.get_name().c_str());
    

So you were using something that was not a char* for a %s format. gcc on Linux should have signaled that error.


From: Arthur Glowacki [notifications@github.com]
Sent: Friday, March 06, 2015 8:49 AM
To: areaDetector/ADCore
Cc: Mark Rivers
Subject: Re: [ADCore] Hdf5 bugfix (#71)

That is strange because I compiled on linux and didn't get any errors.

The writeHdfConstDatasets() function is not needed anymore, I forgot to remove it.


Reply to this email directly or view it on GitHubhttps://github.com//pull/71#issuecomment-77570189.

@aglowacki
Copy link
Copy Markdown
Author

I have asyn-4-22 and didn't get the error when compiling for linux-x86_64-debug

@MarkRivers
Copy link
Copy Markdown
Member

I don't understand why you are not getting an error. This is what I get with commit e95bd3e of NDFileHDF5.cpp, gcc version 4.6.3, asyn R4-26.

corvette:ADCore/ADApp/pluginSrc>make
make -C O.linux-x86_64 -f ../Makefile TOP=../../.. T_A=linux-x86_64 install
make[1]: Entering directory `/home/epics/devel/areaDetector-2-1/ADCore/ADApp/pluginSrc/O.linux-x86_64'

/usr/bin/g++ -c -D_POSIX_C_SOURCE=199506L -D_POSIX_THREADS -D_XOPEN_SOURCE=500 -D_X86_64_ -DUNIX -D_DEFAULT_SOURCE -Dlinux -D_REENTRANT -O3 -g -Wall -m64 -fPIC -MMD -I. -I../O.Common -I. -I.. -I../../../include/os/Linux -I../../../include -I/corvette/usr/local/epics/base-3.14.12.4/include/os/Linux -I/corvette/usr/local/epics/base-3.14.12.4/include -I/corvette/home/epics/devel/asyn-4-26/include -I/corvette/home/epics/devel/areaDetector-2-1/ADBinaries/include -I/corvette/home/epics/devel/areaDetector-2-1/ADCore/include -I/usr/local64/include -I/usr/include/libxml2 ../NDFileHDF5.cpp
../NDFileHDF5.cpp: In member function 'void NDFileHDF5::writeHdfAttributes(hid_t, hdf5::Element_)':
../NDFileHDF5.cpp:510:13: error: cannot pass objects of non-trivially-copyable type 'std::string {aka struct std::basic_string}' through '...'
../NDFileHDF5.cpp:510:13: warning: format '%s' expects argument of type 'char_', but argument 7 has type 'std::string {aka std::basic_string}' [-Wformat]
../NDFileHDF5.cpp: In member function 'hid_t NDFileHDF5::writeHdfConstDataset(hid_t, hdf5::Dataset_)':
../NDFileHDF5.cpp:577:1: warning: control reaches end of non-void function [-Wreturn-type]
make[1]: *_* [NDFileHDF5.o] Error 1
make[1]: Leaving directory `/home/epics/devel/areaDetector-2-1/ADCore/ADApp/pluginSrc/O.linux-x86_64'
make: *** [install.linux-x86_64] Error 2


From: Arthur Glowacki [notifications@github.com]
Sent: Friday, March 06, 2015 9:39 AM
To: areaDetector/ADCore
Cc: Mark Rivers
Subject: Re: [ADCore] Hdf5 bugfix (#71)

I have asyn-4-22 and didn't get the error when compiling for linux-x86_64-debug


Reply to this email directly or view it on GitHubhttps://github.com//pull/71#issuecomment-77579129.

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.

3 participants