Project

General

Profile

Actions

Bug #3834

closed

crushtool really really hates \r

Added by Dan Mick over 11 years ago. Updated almost 7 years ago.

Status:
Resolved
Priority:
High
Assignee:
Category:
-
Target version:
-
% Done:

0%

Source:
Development
Tags:
Backport:
Regression:
No
Severity:
3 - minor
Reviewed:
Affected Versions:
ceph-qa-suite:
Component(RADOS):
Pull request ID:
Crash signature (v1):
Crash signature (v2):

Description

Spent a long time trying to figure out why a crush map wouldn't compile; finally got it to no differences at all, even whitespace, according to every tool I used.

That's because the only remaining difference was EOL: \r\n vs. \n. ARGH.

It turns out a small change makes crushtool insensitive to such things:

--- a/src/crush/CrushCompiler.cc
+++ b/src/crush/CrushCompiler.cc
@@ -686,7 +686,7 @@ string CrushCompiler::consolidate_whitespace(string in)

   bool white = false;
   for (unsigned p=0; p<in.length(); p++) {
-    if (in[p] == ' ' || in[p] == '\t') {
+    if (in[p] == ' ' || in[p] == '\t' || in[p] == '\r') {
       if (white)
        continue;
       white = true;
Actions #1

Updated by Sage Weil over 11 years ago

Ha! Sorry about htat. Maybe iswhite() (or wahtever that helper is) would be best here?

Actions #2

Updated by Ian Colle over 11 years ago

  • Assignee set to Dan Mick
  • Priority changed from Normal to High
Actions #3

Updated by Dan Mick over 11 years ago

Well isspace() would catch newline too, which I think we don't want, so it'd be iswhite(c) && c != '\n', which I'm not sure is better. OTOH that would also catch formfeed, and I can imagine someone
deciding to paginate a crushmap...

Actions #4

Updated by Dan Mick over 11 years ago

  • Status changed from New to Resolved

commit:e776b63dd5c540a6f49b03b67e72a1f4636a74fd

Actions #5

Updated by Greg Farnum almost 7 years ago

  • Project changed from Ceph to RADOS
  • Category deleted (10)
Actions

Also available in: Atom PDF