Bug #577

Prevent AddColumn(0, ...)

Added by Iñaki Zabala over 11 years ago. Updated over 11 years ago.

Status:ApprovedStart date:11/18/2013
Priority:ImmediateDue date:
Assignee:Iñaki Zabala% Done:

0%

Category:GridCtrlSpent time:-
Target version:-

Description

Now calling GridCtrl::AddColumn(const char *name, int size, bool idx) with name = 0 will crash the program as it calls ToLower(name).

The reason is that AddColumn() calls FromUtf8(), that calls utf8len(), that calls strlen, and strlen(NULL) raises an exception.

History

#1 Updated by Miroslav Fidler over 11 years ago

I think this is expected behaviour. You should not pass NULL anywhere where data are expected. That is why C-library functions crash on this too.

I agree that we have an exception to the rule with String (String(NULL) is empty string). Perhaps it was a mistake...

#2 Updated by Iñaki Zabala over 11 years ago

Hello Mirek

GridCtrl now accepts AddColumn() with char *name = NULL in other places like:
- 

GridCtrl::ItemRect& GridCtrl::AddColumn(const Id id, const char *name, int size, bool idx)
{
    return AddColumn(name ? name : (const char *) ~id, size, idx).Alias(id);
}

- 

void GridCtrl::SetColCount(int n, int size)
{
    Reset();
    for(int i = 0; i < n; i++)
        AddColumn(NULL, size, false);
}

- 

ItemRect& InsertColumn(int pos, const char *name = NULL, int size = -1, bool idx = false);
ItemRect& AddColumn(const Id id, const char *name = NULL, int size = -1, bool idx = false);
ItemRect& AddColumn(const char *name = NULL, int size = -1, bool idx = false);

This problem happens not only in AddColumn() but in InsertColumn(), that has another ToLower(name).

#3 Updated by Zbigniew Rebacz over 11 years ago

  • Category set to GridCtrl

I think we should fast fix this issue, because now GridCtrl is useless. I cannot run any of my programs that use GridCtrl :(

#4 Updated by Iñaki Zabala over 11 years ago

Today GridCtrl is still broken.

Sorry for this statement but, this code should be fixed today. No excuses.

#5 Updated by Miroslav Fidler over 11 years ago

  • Status changed from New to Ready for QA
  • Assignee changed from Daniel Kos to Iñaki Zabala

Fixed in the most trivial way ( = ""), I guess it is enough here..

#6 Updated by Iñaki Zabala over 11 years ago

  • Status changed from Ready for QA to In Progress

That is perfect.

Some details related with this that should have to be fixed are:

- DropGrid/DropGrid.h ==> GridCtrl::ItemRect& AddIndex(const char *name = NULL);

Other places in the code to check are:
- Core/profile.h ==> TimingInspector(const char *name = NULL)
- Core/Debug.cpp TimingInspector::TimingInspector(const char *name) ==> name = _name ? _name : "";

- Core/util.h ==> LoadFromFile(T& x, const char *name = NULL, int version = Null)

StoreToFile(T& x, const char *name = NULL, int version = Null)

- Core/Stream.cpp > String Cfgname(const char *file) {
return file ? String(file) : ConfigFile();
}

- Core/Value.h > static void Register(const char *name = NULL);
> static void SvoRegister(const char *name = NULL);

- Core/Xmlize.h ==> String StoreAsXML(const T& data, const char *name = NULL)
bool StoreAsXMLFile(Callback1<XmlIO> xmlize, const char *name = NULL, const char *file = NULL);
bool StoreAsXMLFile(T& data, const char *name = NULL, const char *file = NULL)

- Core/Xmlize.cpp ==> bool StoreAsXMLFile(Callback1<XmlIO> xmlize, const char *name, const char *file) {
return SaveFile(sXMLFile(file), StoreAsXML(xmlize, name ? (String)name : GetExeTitle()));
}

#7 Updated by Zbigniew Rebacz over 11 years ago

Can we also fix DropGrid?

#8 Updated by Iñaki Zabala over 11 years ago

  • Priority changed from High to Immediate

In addition, in GridCtrl.h, line 1247, there is an AddColumn(0, 0, true) that should have to be AddColumn("", 0, true).

My main application has hanged because of it.

As it would have to be done before, the priority has been raised to Immediate.

#9 Updated by Iñaki Zabala over 11 years ago

  • Assignee changed from Iñaki Zabala to Daniel Kos

#10 Updated by Iñaki Zabala over 11 years ago

  • Assignee changed from Daniel Kos to Iñaki Zabala

#11 Updated by Iñaki Zabala over 11 years ago

GridCtrl.h, line 790
ItemRect& DoAvg(const char *s = "") { sop = SOP_AVG; sopfrm = s; return *this; }
ItemRect& DoSum(const char *s = "") { sop = SOP_SUM; sopfrm = s; return *this; }
ItemRect& DoMin(const char *s = "") { sop = SOP_MIN; sopfrm = s; return *this; }
ItemRect& DoMax(const char *s = "") { sop = SOP_MAX; sopfrm = s; return *this; }
ItemRect& DoCount(const char *s = "") { sop = SOP_CNT; sopfrm = s; return *this; }

GridCtrl.h, line 1247
AddColumn("", 0, true)

GridCtrl.cpp, line 1868
AddColumn("", size, false);

DropGrid.h, line 129
GridCtrl::ItemRect& AddIndex(const char *name = "");

#12 Updated by Iñaki Zabala over 11 years ago

  • Status changed from In Progress to Patch ready

#13 Updated by Miroslav Fidler over 11 years ago

  • Status changed from Patch ready to Ready for QA

Well, looks like you have already commited all changes. Can we close this now?

#14 Updated by Iñaki Zabala over 11 years ago

  • Status changed from Ready for QA to Approved

Yes

Also available in: Atom PDF