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

Remove dfi.h from the source #431

Merged
merged 3 commits into from
Sep 13, 2023
Merged

Remove dfi.h from the source #431

merged 3 commits into from
Sep 13, 2023

Conversation

derobins
Copy link
Member

NOTE This is a deployed header file!

This header file is declared internal, but is unused in the library and is undocumented in the RM, UG, and DG.

*NOTE* This is a deployed header file!

This header file is declared internal, but is unused in the library
and is undocumented in the RM, UG, and DG.
@derobins derobins added Component - C Library Core C library issues Type - Deprecation / Removal We strive for backward-compatibility, so it's worth noting this separately Priority - 2. Medium ⏹ It would be nice to have this in the next release labels Sep 12, 2023
@derobins derobins marked this pull request as draft September 12, 2023 22:53
@derobins
Copy link
Member Author

derobins commented Sep 12, 2023

Some of the macros in this file are used in other, unused macros. I will track those down and remove them as well.

Update: DONE (ready for review)

@derobins derobins marked this pull request as ready for review September 12, 2023 23:17
@@ -52,8 +52,7 @@
* Note that rseq does its own compression, because that is part of
* the interactive color raster protocol
* The space needed for compression and decompression can be allocated
* statically or dynamically, depending on the DF_DYNAMIC flag, and
* for entire image or part of it (reused) depending on availability
* for the entire image or part of it (reused) depending on availability
Copy link
Member Author

Choose a reason for hiding this comment

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

It's always dynamic in the code below and DF_DYNAMIC is not used anywhere

/*-----------------------------------------------------------------------------
* File: dfi.h
* Purpose: HDF internal header file
* Invokes: stdio.h, sys/file.h
Copy link
Member Author

Choose a reason for hiding this comment

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

Note "internal"...

return (ret); \
} \
}

Copy link
Member Author

Choose a reason for hiding this comment

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

These are also unused and invoke the dfi.h macros. Since they are located in a C file, there is no risk if we remove them.

@@ -116,7 +116,6 @@ typedef short hdf_file_t;
#define HI_SEEK(x, y) mlseek(x, (int32)y, 0)
#define HI_SEEKEND(x) mlseek(x, 0L, 2)
#define HI_TELL(x) mlseek(x, 0L, 1)
#define DF_OPENERR(f) ((f) == -1)
#define OPENERR(f) (f < 0)
#endif /* FILELIB == MACIO */
Copy link
Member Author

Choose a reason for hiding this comment

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

I'll remove code protected by MACIO in a separate commit. I don't think we need to support MacOS 9 in 2023...


* dfi.h - Declared private, unused, undocumented in UG/RM/DG, has been
removed from the library entirely.
Copy link
Member Author

Choose a reason for hiding this comment

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

I'll expand this list in future commits as we move private headers out of the deploy list

@derobins derobins merged commit 2c3b99b into HDFGroup:master Sep 13, 2023
44 checks passed
@derobins derobins deleted the yank_dfi_h branch March 3, 2024 20:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component - C Library Core C library issues Priority - 2. Medium ⏹ It would be nice to have this in the next release Type - Deprecation / Removal We strive for backward-compatibility, so it's worth noting this separately
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants