What's wrong with this code

Question

Somebody told me that this piece of code has some serious issues but I have not been able to get my head around such issues. Can you guys please educate me on this?

static char BASED_CODE szFilter[] = "HTML Files (*.xls)|*.xls|All Files (*.*)|*.*||";

const char* filter = "HTML Files (*.xlsx)|*.xlsx|All Files (*.*)|*.*||";
size_t length = strlen(filter);
strcpy_s(szFilter, length + 1, filter);

If you're really using C++, consider also using C++ strings. — sarnold, Apr 19 '11 at 11:50
What is BASED_CODE? String literals have type `const char*`. — pmr, Apr 19 '11 at 11:51
Perhaps the fact that the code asserts that .xls and .xlsx extensions are used for HTML files is another problem! ;-) — David Heffernan, Apr 19 '11 at 11:54

score 2 · Answer 1 · answered Apr 19 '11 at 11:47

2

Well, the buffer overrun leaps out at me – szFilter isn't big enough to receive filter.

Why not use std::string since you are using C++? That makes these issues vanish.

answered Apr 19 '11 at 11:47

David Heffernan

601,492
42
1,072
1,490

+1 for a funny way to state the (not so) obvious (one could easily miss the *.xlsx vs. *.xls) – Paul Groke Apr 19 '11 at 11:50

score 1 · Answer 2 · answered Apr 19 '11 at 11:49

1

The second parameter of strcpy_s() should be the size of the destination buffer; you've given it the size of the input string.

But as you're working in C++, you should avoid strcpy() (etc.) entirely, and use std::string.

answered Apr 19 '11 at 11:49

Oliver Charlesworth

267,707
33
569
680

score 0 · Answer 3 · answered Apr 19 '11 at 11:51

0

szFilter is shorter than filter, so there is not enough place to copy filter to szFilter. You current code has undefined behaviour.

answered Apr 19 '11 at 11:51

pajton

15,828
8
54
65

What's wrong with this code

3 Answers3