On Sep 4, 1:40 pm, "Joshua Taylor" <[EMAIL PROTECTED]> wrote: > I was thinking that calling va_copy *is* the portable way to do va_copy, as > least as of C99. I was a little worried about using this newfangled C99 > feature in the patch, then I remembered that the standard is almost a decade > old. People who haven't upgraded their compiler in 10 years are unlikely to > be pulling the latest log4cpp patches and building them. Surely the lack of > va_copy wouldn't be the only problem with building Hypertable (or even just > log4cpp) in such an outdated environment.
C99 is not necessarily implied by a modern C++ compiler (the current standard is 1998). One has to futz with compiler/platform dependent options to enable va_copy. > My patch hasn't actually been accepted, and there are a number of > alternatives. The va_copy path could be conditional on va_copy existence, > and the code could fall back to home-brew va_copy or just simply clipping > the log message otherwise. In fact, always clipping the log message would > be fine with me too. It's the crashing I'm not so happy about. :) > > Also, how would you cap the log message size for Hypertable? Pre-format > everything using your format() function and then pass the resulting string > into log4cpp? That would work, but I think it would be less efficient than > va_copy, since it would wind up formatting each log message twice, whereas > va_copy usually just copies a couple of pointers (I think). Yeah, String::format would work for HT_*F() macros, which is not the most efficient way to cap huge (> 1k) messages (causes at least one large malloc and then a substr), but if we call the log4cpp log() interface instead of logva(), it's a wash for small messages (<1k). HT_*OUT << ... << HT_END macros already uses FixedOstream to cap the size of the log message without extra malloc. __Luke > On Thu, Sep 4, 2008 at 12:11 PM, Luke <[EMAIL PROTECTED]> wrote: > > > You're right. The API is different. It's a headache to do portable > > va_copy though. I think the easiest workaround for Hypertable is to > > cap the message size to 1000. > > > On Sep 4, 11:18 am, "Joshua Taylor" <[EMAIL PROTECTED]> wrote: > > > I don't think va_start would work in this situation. > > > > Hypertable's format() is: > > > String format(char const * fmt, ...); > > > > log4cpp's vform() is: > > > string vform(char const * fmt, va_list args); > > > > Someone else has already called va_start() and passed the result to > > > vform(). After vform() returns, the caller does a va_end(). I don't > > think > > > you can portably reuse va_start in this situation and ignore the passed > > in > > > argument vector. Also, I don't think log4cpp can be refactored to avoid > > > this type of problem since the public API allows external code to pass in > > > va_lists, for example in Category::logva(). > > > Well, the standard just says you needed intervening va_end before you > > can use > > > > On Thu, Sep 4, 2008 at 10:36 AM, Luke <[EMAIL PROTECTED]> wrote: > > > > > I just took a look at the patch. It seems to me that using va_start > > > > instead of va_copy would be more portable, faster as well > > > > (Hypertable::String::format use this and extensively tested with > > > > various buffer sizes.) > > > > > On Aug 29, 6:43 pm, "Joshua Taylor" <[EMAIL PROTECTED]> wrote: > > > > > I've been having problems with RangeServer crashing in log4cpp when > > it > > > > has > > > > > long log messages. The one that has been killing me is in > > > > > split_notify_master when the row keys involved are in the 400-900 > > byte > > > > > range. > > > > > > When log4cpp tries to format something over 1023 characters, it blows > > up. > > > > > It has some code in place to handle long log messages, but apparently > > it > > > > has > > > > > never been tested because it is broken. The code has been broken for > > the > > > > > last 5 years, up to and including the recent 1.0 release of log4cpp. > > > > I've > > > > > submitted a test case and fix to the SourceForge project. I've > > attached > > > > the > > > > > fix, in case other people are having the same problem. The patch is > > > > taken > > > > > against the 1.0 code base, but according to their CVS, these files > > > > haven't > > > > > changed in 5 years, so you can probably apply it to older versions > > too. > > > > > > Josh > > > > > > 0003-Fixed-bug-in-StringUtil-vform.patch > > > > > 1KViewDownload --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Hypertable Development" group. To post to this group, send email to [email protected] To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/hypertable-dev?hl=en -~----------~----~----~----~------~----~------~--~---
