[program_options] Patch to fix 64bit error and warning
The attached patch fixes a 64 bit portability problem where std::string::size_type is assigned to unsigned, which is shorter than size_t on x86-64 and so will be truncated. This means the following comparison to std::string::npos is always false. Patch also prevents a warning with GCC; since the compiler doesn't know that boost::throw_exception never returns it thinks that cmdline::translate_property() can reach the end of the function without returning. The patch simply removes the "else" so that throw_exception is always called if control reaches the end of the function. A better solution might be to mark boost::throw_exception() with __attribute__((unused)) for GCC. The first change is required, the second is optional. jon -- "More computing sins are committed in the name of efficiency (without necessarily achieving it) than for any other single reason - including blind stupidity." - W.A. Wulf
Hi Jonathan,
The attached patch fixes a 64 bit portability problem where std::string::size_type is assigned to unsigned, which is shorter than size_t on x86-64 and so will be truncated. This means the following comparison to std::string::npos is always false.
Applied. Interesting, I'd say that a std::string of more that 4GB is a terrible idea, performance wise, as many operations are linear on string size. Do you know why it's decided to make size_type 64bit?
Patch also prevents a warning with GCC; since the compiler doesn't know that boost::throw_exception never returns it thinks that cmdline::translate_property() can reach the end of the function without returning. The patch simply removes the "else" so that throw_exception is always called if control reaches the end of the function. A better solution might be to mark boost::throw_exception() with __attribute__((unused)) for GCC.
Shouldn't it be __attribute((noreturn)) ? But anyway, that part of code is rewritten in my local copy and that function is removed. Thanks! - Volodya
On Wed, Mar 23, 2005 at 02:12:03PM +0300, Vladimir Prus wrote:
Hi Jonathan,
The attached patch fixes a 64 bit portability problem where std::string::size_type is assigned to unsigned, which is shorter than size_t on x86-64 and so will be truncated. This means the following comparison to std::string::npos is always false.
Applied. Interesting, I'd say that a std::string of more that 4GB is a terrible idea, performance wise, as many operations are linear on string size. Do you know why it's decided to make size_type 64bit?
size_t must be 64-bit or you couldn't request >4GB from new and malloc. IIRC std::string::size_type must be std::size_t. AFAIK there's no requirement that std::string::max_size() == 4GB, even though the return type is large enough to hold that value, so an implementation could restrict strings to reasonable sizes, it just can't use a 32bit type to report the size.
Patch also prevents a warning with GCC; since the compiler doesn't know that boost::throw_exception never returns it thinks that cmdline::translate_property() can reach the end of the function without returning. The patch simply removes the "else" so that throw_exception is always called if control reaches the end of the function. A better solution might be to mark boost::throw_exception() with __attribute__((unused)) for GCC.
Shouldn't it be
__attribute((noreturn))
duh! yes, of course. My fingers took a short cut that avoided my brain and typed something else I was thinking about! jon -- VOTE, v. The instrument and symbol of a free man's power to make a fool of himself and a wreck of his country. - Ambrose Bierce, 'The Devil's Dictionary'
On Wed, Mar 23, 2005 at 11:38:53AM +0000, Jonathan Wakely wrote:
On Wed, Mar 23, 2005 at 02:12:03PM +0300, Vladimir Prus wrote:
Applied. Interesting, I'd say that a std::string of more that 4GB is a terrible idea, performance wise, as many operations are linear on string size. Do you know why it's decided to make size_type 64bit?
size_t must be 64-bit or you couldn't request >4GB from new and malloc. IIRC std::string::size_type must be std::size_t.
AFAIK there's no requirement that std::string::max_size() == 4GB,
that should read "max_size() > 4GB" I'm typing a right old load of rubbish today, sorry! jon -- "Only two things are infinite: the universe and human stupidity, and I'm not sure about the former." - Albert Einstein
participants (2)
-
Jonathan Wakely -
Vladimir Prus