H5fget_name_f len_trim fix

[jhaiduce]

h5fget_name_f was calling the intrinsic len_trim to determine the length of the passed buffer. This can cause problems if the passed buffer contains trailing whitespace (in which case the h5fget_name_f will limit itself to the length of the non-whitespace characters currently in the passed buffer, even if the buffer's capacity is larger), or if the passed buffer was freshly allocated (in which case the resulting buffer length is unpredictable).

The pull request replaces the call to len_trim with a call to len, which returns the total buffer length regardless of the passed contents of the buffer, and is safe to call on a freshly allocated (i.e. unassigned) buffer.

This fixes #825.

[epourmal]

Please provide testing code that shows the problem before the fix and passes after the fix.
Thank you!

[jhaiduce]

Please provide testing code that shows the problem before the fix and passes after the fix.
Thank you!

Here's some example code that fails on the current development branch and passes once this PR is applied:

program h5test
  use hdf5
  use, intrinsic :: iso_fortran_env, only: error_unit

  implicit none

  integer::ierr

  integer(HID_T)::h5file_h
  integer(SIZE_T)::size

  character(80)::filename
  character(*), parameter::expected_filename = "test.h5"

  integer::i

  call h5open_f(ierr)

  call h5fcreate_f(expected_filename, H5F_ACC_TRUNC_F, h5file_h, ierr)

  do i = 1,80
     filename(i:i) = ' '
  end do

  call h5fget_name_f(h5file_h, filename, size, ierr)

  if(size/=len(expected_filename)) then
     write(error_unit,*) 'Wrong filename length'
     error stop
  end if

  if(filename(1:size)/=expected_filename) then
     write(error_unit,*) 'Filename does not match'
     error stop
   end if

  call h5fclose_f(h5file_h, ierr)

end program h5test

I also added some changes to this PR that modify tH5F.F90 so that the test subroutine reopentest would fail with the previous implementation, but passes with the fix implemented in this PR.

[brtnfld]

The issue is the majority of Fortran users expect the trailing spaces to be trimmed if they are creating a character string that is larger than the actual filename, having a filename with trailing white space is not common. This also mimics Fortran standard OPEN, which also does not necessarily create a filename with trailing whitespace.

A better solution would be to add an optional len parameter for the filename. But that is not needed because a user can use C_NULL_CHAR to preserve the trailing white space.

CHARACTER(*), PARAMETER::expected_filename = "test.h5 "//C_NULL_CHAR

C has a different behavior because the null terminator determines the string size.

[brtnfld]

I see the issue, I had read the issue as trailing white space in the filename. Let me look into it.

[brtnfld]

Also, change the declaration (line 827):

CHARACTER(LEN=*), INTENT(OUT) :: buf

[brtnfld]

fixed in new PR.

[brtnfld]

I would leave in the old test and add a new test:

     CALL h5dopen_f(reopen_id, dsetname, dset_id, error)
          CALL check("h5dopen_f",error,total_error)
     !
     !Get file name from the dataset identifier
     !
     ! Use an uninitialized buffer
     CALL h5fget_name_f(dset_id, file_name, name_size, error)
     CALL check("h5fget_name_f",error,total_error)
     IF(name_size .NE. LEN_TRIM(fix_filename))THEN
        WRITE(*,*) "  file name size obtained from the dataset id is incorrect"
        total_error = total_error + 1
     ENDIF
     IF(file_name(1:name_size) .NE. TRIM(fix_filename)) THEN
        WRITE(*,*) "  file name obtained from the dataset id is incorrect"
        total_error = total_error + 1
     END IF
     ! Use an initialized buffer
     file_name(:) = " "
     CALL h5fget_name_f(dset_id, file_name, name_size, error)
     CALL check("h5fget_name_f",error,total_error)
     IF(name_size .NE. LEN_TRIM(fix_filename))THEN
        WRITE(*,*) "  file name size obtained from the dataset id is incorrect"
        total_error = total_error + 1
     ENDIF
     IF(file_name(1:name_size) .NE. TRIM(fix_filename)) THEN
        WRITE(*,*) "  file name obtained from the dataset id is incorrect"
        total_error = total_error + 1
     END IF

     !
     !Read the dataset.
     !

[brtnfld]

The size should always be correct, just the filename would be wrong since the buffer size given was zero.

[jhaiduce]

I would leave in the old test and add a new test

I pushed some changes reverting my edits to reopentest and adding a new test focusing on h5fget_name_f.

[brtnfld]

@jhaiduce, should I make the changes and issue a new PR?

[brtnfld]

I would suggest simplifying this by just opening the root group "/" and using the group id instead of going through the trouble of creating a dataset.
call h5gopen_f(file_id,"/",g_id,ierr)

[jhaiduce]

@jhaiduce, should I make the changes and issue a new PR?

Sounds good to me, thanks!

[brtnfld]

Reworked PR, new PR #972