[Server] Fix an include in OssCsi #1511
Conversation
|
Hi @smithdh and thanks! :) while maybe out-side of your scope of work, maybe it would useful to convert all inttypes.h usage to cinttypes? @simonmichal what do you think? |
|
I'm not sure what you mean by moving from inttypes.h to cinttypes.h as we
always use either stdint.h or cstdint. That said, using fixed sized
integers it generally something to be avoided. The reason is that ints,
longs, and long longs mean different things on different platforms and
when you force a particular width on a platform that doesn't natively
support it runs pretty slow. We are seeing more movement of the xroot
package to platforms that have diffent definitions (width wise) for the
standard "int" types which is OK as long we we don't force a particular
width when it's not specifically needed.
Andy.
…On Fri, 10 Sep 2021, Adrian Sevcenco wrote:
Hi @smithdh and thanks! :) while maybe out-side of your scope of work, maybe it would useful to convert all inttypes.h usage to cinttypes? @simonmichal what do you think?
Thanks a lot!
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
#1511 (comment)
########################################################################
Use REPLY-ALL to reply to list
To unsubscribe from the XROOTD-DEV list, click the following link:
https://listserv.slac.stanford.edu/cgi-bin/wa?SUBED1=XROOTD-DEV&A=1
|
|
@abh3 so, beside of the sl6 error that we have (and this PR is fixing it ), it is not a good practice to use c headers in c++ code, and AFAIK there is always a recommendation to switch to usage of c++ includes (so |
|
Hi Adrian,
Understood. I would prefer that we replace all occurrences of either
header file with <cstdint>. That provides the best compatibility and
adheres to what is expected in C++ programs. Otherwise, I totally agree
with you.
Andy
…On Fri, 10 Sep 2021, Adrian Sevcenco wrote:
@abh3 so, beside of the sl6 error that we have (and this PR is fixing it ), it is _not_ a good practice to use c headers in c++ code, and AFAIK there is always a recommendation to switch to usage of c++ includes (so `we always use either stdint.h or cstdint` is actually the problem that i'm talking about.).
regarding this present issue, the problem is that c header did not correctly worked on gcc 7.3.0 and the problem is fixed with c++ header. given the wide usage of inttypes.h in xrootd code i gave the suggestion for the wide fix (not just in this file, just use cinttypes everywhere).
Any other considerations, future developments, whole code refactoring and architecture decisions can be always done in future PRs i would say..
--
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub:
#1511 (comment)
|
Compiling XrdOssCsiPages.hh was breaking builds on sl6, even when using when a compiler with c++14 support. This was reported by an lhc experiment. The cause was that in this build inttypes.h did not define macros such as PRIx64. On sl6, setting __STDC_FORMAT_MACROS before the include does cause it to define the macros; but the solution in this PR is to use the c++11 version of this header, cinttypes.