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.
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). 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 -~----------~----~----~----~------~----~------~--~---
