move silo_exports logic to config.h
Nobody has claimed this yet.
- Dominant language
- C
- Stars
- 45
- Forks
- 27
- Avg merge
- 12h 34m
- Merged PRs (30d)
- 4
Description
We're including 'silo_exports.h' in score.h, a PDBLite source. I don't think we should do that. But, I think I am hung up on the name of the include file having 'silo' in it. All silo_exports.h really has in it is the windowsspecific logic to correctly export symbols with the SILO_API macro. I thought about putting this into config.h instead. That would be fine except that silo_exports logic needs to appear in silo.h too. So, that would mean that #include'ing silo.h in a source file would also include Silo's config.h. That would be bad as that would mean symbols specific to internal compilation of the Silo library then pollute any Silo client's CPP namespace. If we could find a way that config.h is essentially 'empty' when its included in a true silo client, then that would fix this issue. I don't understand enough about silo_exports to know whether its a message down to the lib to do something with SILO_API'd symbols or a message up to caller of Silo library telling it which symbols are really defined for it to reference.
-----------------------REDMINE MIGRATION-----------------------
This ticket was migrated from Redmine. As such, not all
information was able to be captured in the transition. Below is
a complete record of the original redmine ticket.
Ticket number: 567
Status: New
Project: VisIt
Tracker: Bug
Priority: Normal
Subject: move silo_exports logic to config.h
Assigned to: -
Category: -
Target version: -
Author: Mark Miller
Start: 01/25/2011
Due date:
% Done: 0%
Estimated time:
Created: 01/25/2011 01:39 am
Updated: 01/25/2011 11:04 pm
Likelihood: 3 - Occasional
Severity: 2 - Minor Irritation
Found in version: 4.8
Impact:
Expected Use:
OS: All
Support Group: Any
Description:
We're including 'silo_exports.h' in score.h, a PDBLite source. I don't think we should do that. But, I think I am hung up on the name of the include file having 'silo' in it. All silo_exports.h really has in it is the windowsspecific logic to correctly export symbols with the SILO_API macro. I thought about putting this into config.h instead. That would be fine except that silo_exports logic needs to appear in silo.h too. So, that would mean that #include'ing silo.h in a source file would also include Silo's config.h. That would be bad as that would mean symbols specific to internal compilation of the Silo library then pollute any Silo client's CPP namespace. If we could find a way that config.h is essentially 'empty' when its included in a true silo client, then that would fix this issue. I don't understand enough about silo_exports to know whether its a message down to the lib to do something with SILO_API'd symbols or a message up to caller of Silo library telling it which symbols are really defined for it to reference.
Comments:
Well, config.h is a bad idea. However, moving it up to the same dir as config.h maybe good idea.
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start by examining silo_exports.h, score.h, silo.h, and config.h to trace which export definitions are needed internally versus by Silo clients. Review the issue's include-pollution concern and determine a header arrangement that preserves SILO_API behavior without exposing internal configuration through public headers. Done means the include relationships are corrected and client-visible headers no longer pollute the C preprocessor namespace.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- c
- Domain
- databases
- Issue type
- Refactor
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Needs clarification
- Newbie friendliness
- 35/100