-1

Okay, I've been dealing with this for two days now, and I can't find a solution.

Problem: I'm trying to set a filter to a File Selection Dialog using Winapi. I'm using GetOpenFileName function to do this. This function uses a structure to set options such as file extension filters. This structure's member called lpstrFilter needs a certain string format. I'm setting that string exactly as Winapi indicates, but for some reason, this string's value changes.

I've got this static const char *:

//This contains string "JPG"
static const char * extensionFilter = v->trabajo10.C_JMV_SelectFile_FileExtension7.GetString();

//This forms a filter string which applies to OPENFILENAME structure.
string sFilter;
sFilter.append("Format: ");
sFilter.append(extensionFilter);
sFilter.push_back('\0');
sFilter.append("*.");
sFilter.append(extensionFilter);
sFilter.push_back('\0');
const char * filter = sFilter.c_str();
ofn.lpstrFilter = filter; //This sets: --> Format: JPG\0*.JPG\0

//This opens the file selection dialog
if (GetOpenFileName(&ofn)==TRUE){
...

The File Selection Dialog looks CORRECTLY, like this:

enter image description here

The joke comes now, I modify the code like this:

//This contains string "JPG"
static const char * extensionFilter = v->trabajo10.C_JMV_SelectFile_FileExtension7.GetString();

if(1){
   //This forms a filter string which applies to OPENFILENAME structure.
   string sFilter;
   sFilter.append("Format: ");
   sFilter.append(extensionFilter);
   sFilter.push_back('\0');
   sFilter.append("*.");
   sFilter.append(extensionFilter);
   sFilter.push_back('\0');
   const char * filter = sFilter.c_str();
   ofn.lpstrFilter = filter; //This sets: --> Format: JPG\0*.JPG\0
}

//This opens the file selection dialog
if (GetOpenFileName(&ofn)==TRUE){
...

And this is the result, THE PROBLEM:

enter image description here

Filter string was modified???

5
  • Had you used a debugger, and inspected the OPENFILENAME structure prior to calling GetOpenFileName, you wouldn't have had to ask this question. This question makes up facts that aren't. Commented Dec 11, 2015 at 12:52
  • @IInspectable I'm not able to use a debugger, the IDE I'm using won't let me. Commented Dec 11, 2015 at 13:37
  • @ProtectedVoid which IDE do you use? and you can always get a debugger separately; both WinDbg and gdb can run independent of an IDE. Commented Dec 11, 2015 at 13:43
  • @andlabs I use CA Plex 6.1. It's a quite old IDE that let's you create multi-platform applications. I have no choice, I must use this IDE. It doesn't even let you define C++ functions. It forces you to program in C++ like it was a simple scripting language. I hope you understand a bit more my limitations. Commented Dec 11, 2015 at 13:59
  • It's not actually the if clause. It's the curly brackets {} Any declaration inside the brackets fall in a different scope, they won't be seen outside the brackets. Commented Dec 12, 2015 at 3:32

3 Answers 3

5
if(1){
   //This forms a filter string which applies to OPENFILENAME structure.
   string sFilter;
   sFilter.append("Format: ");
   sFilter.append(extensionFilter);
   sFilter.push_back('\0');
   sFilter.append("*.");
   sFilter.append(extensionFilter);
   sFilter.push_back('\0');
   const char * filter = sFilter.c_str();
   ofn.lpstrFilter = filter; //This sets: --> Format: JPG\0*.JPG\0
}

The sFilter variable has a lifetime that ends when the block in which it declared ends. The pointer returned by sFilter.c_str() is valid until sFilter is modified or destroyed.

You are using this pointer after it has become invalidated. This is the same problem as you had yesterday, which I guessed at in comments to the question. This is why you need to show a full MCVE. This question also looks to be a duplicate of the one that you asked a week ago: Winapi GetOpenFileName Extension Filter not working. I suggest that you take some time to make sure that you fully appreciate the validity of the value returned by c_str().

You must ensure that sFilter lives until after you have finished with the pointer. Declare sFilter in an outer block to ensure that.

Sign up to request clarification or add additional context in comments.

5 Comments

I understand the problem now, thanks. But this is forcing me to create a variable in an outer block, which it means it will be defined even if I don't need it? What If I'll only use it if a condition is met? Wasn't it logical to create it when the condition is met? Thanks again.
Yes, I can see why you wrote the code that way. It's an easy trap to fall in to.
@ProtectedVoid The text is conditionally created, so it must be conditionally destroyed. And clearly it must be destroyed later than the end of the original conditional. So some object outside the original conditional must at least remember whether the text must be destroyed. Having sFilter declared (but default empty) outside the conditional is the simple way to accomplisg that.
@JSF You can clear a variable's content but the variable is declared, making your code inefficient if it's not used. What if I had 100 variables declared and they would only be used if a condition is met, all the memory allocation would be useless. I'm starting in C++ and my intention is to increase my knowledge. Thanks.
If it were 100 conditional variables, it would be worth more effort to reduce the footprint of the unused case. Here that footprint could be reduced to a single bool telling whether ofn.lpstrFilter "owns" the thing it points to. That bool would need to be outside the conditional and the code inside the conditional would need to be different such that the text buffer is owned by that pointer. A default empty string is pretty low cost. So reducing the footprint of "unused" down to the minimum of one bool is not worth the large effort required.
2

The problem is that you have a variable that fell out of scope

if(1){
   string sFilter;

   // ... code

   // Right here
   const char * filter = sFilter.c_str();      
   ofn.lpstrFilter = filter;
}

After that block ends filter falls out of scope, so ofn.lpstrFilter has a dangling pointer.

1 Comment

Actually it makes no difference that filter falls out of scope. The dangling pointer is because sFilter fell out of scope. (I'm sure you knew that and even quoted code to make it clear. But your inexact description might confuse someone.) Moving filter to a wider scope wouldn't help, nor would leaving it in the narrow scope hurt. Moving just sFilter to a wider scope would fix the problem.
0

Answering ProtectedVoid's concern about declaring an unused object: Imagine a std::string was an expensive object and the condition was unlikely. You don't want the object constructed unless the condition is true. But then it must last beyond the scope of the condition. So we use the fact that a default constructed unique_ptr is much cheaper than a default constructed string:

std::unique_ptr<std::string> scope_extender;
if( something unlikely ){
       //This forms a filter string which applies to OPENFILENAME structure.
       std::string* sFilter = new std::string;
       scope_extender.reset( sFilter );
       sFilter->append("Format: ");
       sFilter->append(extensionFilter);
       sFilter->push_back('\0');
       sFilter->append("*.");
       sFilter->append(extensionFilter);
       sFilter->push_back('\0');
       const char * filter = sFilter->c_str();
       ofn.lpstrFilter = filter; //This sets: --> Format: JPG\0*.JPG\0
    }

Obviously I don't want to imply std::string is expensive enough to construct to be worth all that trouble. But some object in a similar situation might be. Also, there was a comment about what if it was many minor objects in one conditional. In that case, you would want a utility struct to hold them all together and the unique_ptr conditionally owns that struct.

1 Comment

Thanks, nice example! I appreciate your effort.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.