I call bools**t

Pages: 12
I had code that was working, when the loop was more complicated (I thought dragging in windows.h would improve the user experience. (Boy was I wrong, at least for what I was trying to do!) Anyways, when I took that out, and simplified it to this:
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
    bool keepgoing = true;
    while (keepgoing)   //Putting this here not only allows for the user to keep generating opcode files for their programs, but makes it easier to
    //process these files (the lines in the files will be passed as constructor value)...
    {
        //While we are in here, we should check for the file's existence (with if and else statements...) and then call the functions in the class...
        cout << "Please enter the filename (without the .txt extension; it will be assumed): ";
        getline(cin,filename);
        filename+=".txt";   //I told you I was assuming text files!
        cout << "Now enter the full path of the file (just click on the box next to the Refresh button on your Window) ";
        getline(cin,directory);
        //Replacing every '\' in the directory string with '/' (if any exist). This is IMPERATIVE because C++ automatically associates '\' with SPECIAL
        //CHARACTERS!
        //The ASCII code for '\' is 0x5C (or 92 in decimal!)
        for (size_t spot = directory.find_first_of((char)92); spot != directory.npos; spot = directory.find_first_of((char)92, spot++) )
        {
            directory[spot] = '/';
        }   //end for
        directory += '/'+filename;  //Appending the filename to the directory (to form full pathname)...
        //}   //end if
        ifstream myfile (directory.c_str());
        if (myfile.is_open())
        {
            //making this while loop to determine heap size for the constructors (there is no other choice)...
            //It is worth noting that stuff from this loop on down could go into its own statement, as it is common to both ways to use this program.
            while (myfile.good())
            {
                getline(myfile, line);
                n++;    //n == size of heap
            }   //end while
            myfile.close(); //This was only necessary for the heap (I had no other choice!)
            myfile.clear();
            keepgoing = false;
        }   //end if
        else
        {
            cout << "Invalid pathname entered. Please try again.\n"<<endl;
        }   //end else
    }   //end while 
The assignment of keepgoing = false; causes the whole program to crash! What is going on!?
why would a simple assignment cause the whole program to crash?

btw: you can also do this in your string: c:\\filepath\\file.txt

the escape character sees the next one as a backslash.
I don't see anything in this incomplete snippet that would cause your code to crash, so I suspect the cause of the crash is elsewhere. But you can be assured the cause is not you assigning false to a variable of type bool.

A couple things though:

//Replacing every '\' in the directory string with '/' (if any exist). This is IMPERATIVE because C++ automatically associates '\' with SPECIAL CHARACTERS! The ASCII code for '\' is 0x5C (or 92 in decimal!)


This is only true in one context: that of string literals. Since you aren't dealing with string literals, there is no need to replace them (and indeed, there isn't any need to replace them in string literals as long as you've escaped them properly. It just makes them a little easier to write.)

The ASCII code for '\' is '\' and you should refer to it that way in code. Magic numbers are something you want to avoid.

Your code would be much easier to read without the comments.
i need to learn how to use try, catch and throw.
It doesn't crash until it gets to the keepgoing = false; statement, and yes, cire, I have already tried your suggestion BEFORE I went to the ASCII numbers.
Last edited on
Where it crashes doesn't necessarily have anything to do with why it crashes.

I have already tried your suggestion BEFORE I went to the ASCII numbers.


And what made you decide to go the obfuscated route?
It not working. It crashes either way, unless I decide to throw ALL THE CLASS BUSINESS in the loop. That's how it was working before. Now that I throw it back in, even though I am using '\\' in place of (char)92, IT STILL CRASHES!! (Obviously, the class business has nothing to do with this.) What the hell is going on?
Last edited on
It not working.


That was probably due to the same mistake I made above -- it's much easier to catch in an IDE.

'\\' not '\' .
spot = directory.find_first_of((char)92, spot++)

Maybe undefined behavior?

If not, the post-fix doesn't happen until the assignment to spot is complete. Thus, your find returns the same position it started with, and then increments it.

Try: spot = directory.find_first_of('\\', spot+ 1)
Last edited on
That fixed a problem, but the whole thing still crashes. This may be a logic error somewhere. I am trying to check if (myfile.is_open()), but then end up closing the file and re-opening it (for the class business, of course, and all this inside the conditional) What it should do if it wasn't able to open the file is just prompt the user to re-enter the filename and path. Maybe there is other way to check if the file was found (besides doing what I did initially and bringing in the windows.h?
Last edited on
OK, I have tweaked my loop a little bit, and it is STILL having problems! (It doesn't do this if the file doesn't exist!!) Here is my updated loop:
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
    while (keepgoing)   //Putting this here not only allows for the user to keep generating opcode files for their programs, but makes it easier to
    //process these files (the lines in the files will be passed as constructor value)...
    {
        //While we are in here, we should check for the file's existence (with if and else statements...) and then call the functions in the class...
        cout << "Please enter the filename (without the .txt extension; it will be assumed): ";
        getline(cin,filename);
        filename+=".txt";   //I told you I was assuming text files!
        cout << "Now enter the full path of the file (just click on the box next to the Refresh button on your Window) ";
        getline(cin,directory);
        //Replacing every '\' in the directory string with '/' (if any exist). This is IMPERATIVE because C++ automatically associates '\' with SPECIAL
        //CHARACTERS! We must, therefore, use '\'!
        for (size_t spot = directory.find_first_of('\\'); spot != directory.npos; spot = directory.find_first_of('\\', spot+1) )
        {
            directory[spot] = '/';
        }   //end for
        directory += '/'+filename;  //Appending the filename to the directory (to form full pathname)...
        ifstream myfile (directory.c_str());
        if (myfile != 0)
        {
            //making this while loop to determine heap size for the constructors (there is no other choice)...
            //It is worth noting that stuff from this loop on down could go into its own statement, as it is common to both ways to use this program.
            while (myfile.good())
            {
                getline(myfile, line);
                n++;    //n == size of heap
            }   //end while
            myfile.close(); //This was only necessary for the heap (I had no other choice!)
            keepgoing = false;
        }   //end if
        else
        {
            cout << "Invalid pathname entered. Please try again.\n"<<endl;
        }   //end else
    }   //end while 

I have even tried while (keepgoing == true) which is the same thing as what I have. (And yes, keepgoing = true before the loop)...
Last edited on
spot = directory.find_first_of((char)92, spot++)

Maybe undefined behavior?


Nope, but in:

spot = directory.find_first_of('\\', spot+ 1)

spot+1 may be outside the acceptable bounds for indexing string. Whereas spot++ in this case would be effectively the same as just spot and couldn't be out of bounds.
Wouldn't spot == directory.npos, then? Plus, I even modify that for loop to
1
2
3
4
5
6
size_t spot;
while (directory.find_first_of('\\') != directory.npos)
{
    spot = directory.find_first_of('\\');
    directory[spot] = '/';
}    //end while 

AND IT STILL CRASHES! (It must be my class business. In fact, I have identified the problem to exist in a function, but have analyzed it and couldn't find the problem.) Should I make a link to my file and then post it here, or should I just post the class, its functions, and the calls to the class here?

Or a better question: Should a function have a private variable (namely, one getting set by the constructor) as its parameter?
Last edited on
Hotice wrote:
Wouldn't spot == directory.npos, then?


No, it wouldn't.

Hotice wrote:
AND IT STILL CRASHES!


There was nothing wrong with the for loop to begin with, aside from it not being a great candidate for a for loop.

cire wrote:
I suspect the cause of the crash is elsewhere.


If you could reduce this to a minimal compilable code snippet that still exhibits the problem, that would probably increase your chances of getting meaningful input.
spot+1 may be outside the acceptable bounds for indexing string. Whereas spot++ in this case would be effectively the same as just spot and couldn't be out of bounds.


I think I disagree: This loop won't go out of bounds because in order for it to continue, spot can not be npos
1
2
3
size_t spot = str.find(' ');
while (spot != std::string::npos)
  spot = str.find(' ', spot +1);  


This loop can (will) go out of bounds:
1
2
3
size_t spot = str.find(' ');
while (spot != std::string::npos)
  spot = str.find(' ', spot++);  


Line three might make spot equal npos, but then it is post-incrimented. The while loop then checks against a spot that has been put past npos.
Last edited on
If you could reduce this to a minimal compilable code snippet that still exhibits the problem, that would probably increase your chances of getting meaningful input.
OK, then. Here is some of my class business, including the function I think is causing the problem:
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
class assemblytoopcode
{
    private:
        int varstart;
        string aline;
        string templabel, tempoperand, tempinstruction;
        vector <int> varsize;
        vector <string> labels, operands, instructions;
        size_t pos;
    public:
        assemblytoopcode();
        assemblytoopcode(string);
        void findlabels(string);    //This function is supposed to find the labels in the file (assembly program)...
        bool checkduplicates(); //This function provides for a less complex way to check for duplicate labels. (Should we end the program if they are
        void findoperands();    //found?)
        void minvariableline();
        string getlabels();
        void setlabels(string);
        string getoperands();
        void setoperands(string);
        void PressEnterToContinue();
};  //end class

assemblytoopcode::assemblytoopcode()    //Just in case we need it!
{
    varstart = 0;
    aline = "";
    templabel = "";
    tempoperand = "";
    tempinstruction = "";
}   //end null constructor

assemblytoopcode::assemblytoopcode(string x)
{
    aline = x;
}   //end parameterized constructor

void assemblytoopcode::findlabels(string aline)
{
    if (aline[0] == '.')
    {
        //It's a comment line or has no label; do nothing...
        aline = "";     //Except delete the whole freaking string! (This could be checked by a conditional for the findoperands()!)
    }   //else if
    else if (aline[0] == '\t')  //Else if the first character is a tab character...
    {
        aline.erase(aline.begin());  //...we delete only that character so that our method in findoperands() will work seamlessly....
    }
    else
    {
        cout <<"Am I going to freeze, yet?"<<endl;
        templabel = aline.substr(0, aline.find_first_of('\t'));     //Extract the label to temporary variable
        cout << "This is what got saved to templabel: "<<templabel<<endl;
        //Erase the label from the parsed line (ease of coding at the price of added algorithmic complexity, of O(l), where l is length of file)
        aline.erase(0,(int)aline.find_first_of('\t'));
        //Store to a vector (if it isn't already!);
        //First, we handle the case that no labels have been found yet...
        if (templabel == "")
        {
            //Do nothing! There is nothing in templabel, so don't push it anywhere!
        }    //end if
        else if (labels.size() == 0)
        {
            labels.push_back(templabel);
        }   //end if
        //Now to take care of the case where there are members already in the vector...
        //I think there exists a less algorithmically complex way of doing this. The less complex way would be to wait until you have scanned ALL LABELS
        //to check them for duplicate. This is less complex because you don't have to push in elements until the final instance. This, however requires
        //a double for-loop where the first would control the first element compared, and the inner for controls the element it is being compared to.
        else
        {
            labels.push_back(templabel);
            for (int j = 0; j < (int)labels.size(); j++) cout <<"labels["<<j<<"] == "<<labels[j]<<endl; //Test line; discard at moment's notice
        }   //end else
    }   //end if
}   //end findlabels 

and its calls after the while (keepgoing) loop in the main():
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
    assemblytoopcode * a = new assemblytoopcode[n];
    ifstream thefile(directory.c_str());

    for (int m = 0; m < n; m++)
    {
        getline(thefile, line);
        a[m] = assemblytoopcode(line);
    }   //end for
    for (int j = 0; j < n; j++) a[n].setlabels(a[j].getlabels());
    cout << "I didn't crash...yet"<<endl;
    for (int m = 0; m < n; m++)
    {
        a[m].findlabels(line);        //I made the function take the line from the file as a directory(just to be on the safe side)...*/
        a[m].findoperands();
    }   //end for
    //On the nth instance, we should have everything put together...
    //...including with spotting any reoccurrences of labels...
    if (a[n].checkduplicates())
    {
        //They have duplicate entries. Abort program...
    }   //end if
    else
    {
        for(int m = 0; m < n; m++) a[n].setoperands(a[m].getoperands());
    }   //end else
    a[n].PressEnterToContinue();
    thefile.close();

Asking my previous question:
Or a better question: Should a function have a private variable (namely, one getting set by the constructor) as its parameter?
could provide a solution to this...
Upon freezing, the program doesn't cout any of the statements in findlabels().
Last edited on
Got it! The first assignment to templabel is making it freeze! But, why? (NOTE: I changed aline[0] to aline.at(0))
Last edited on
I think I disagree: This loop won't go out of bounds because in order for it to continue, spot can not be npos

1
2
3
size_t spot = str.find(' ');
while (spot != std::string::npos)
  spot = str.find(' ', spot +1);  


That's not really relevant. You're not feeding find spot. You're feeding it spot+1, which can be out of range.

Consider this string: "string "

After the first iteration of the loop, spot is 6.

On the second iteration, because spot is not std::string::npos, we do:

spot = str.find( ' ', 7 ) ;

And 7 is not a valid index into str.

This loop can (will) go out of bounds:
1
2
3
size_t spot = str.find(' ');
while (spot != std::string::npos)
  spot = str.find(' ', spot++);  

Line three might make spot equal npos, but then it is post-incrimented. The while loop then checks against a spot that has been put past npos.


Side effects for arguments to functions are sequenced before the function call. Spot is guaranteed to be incremented before the assignment, therefore this cannot and will not go out of bounds. It will, however, result in an infinite loop if the ' ' is found.
Got it! The first assignment to templabel is making it freeze! But, why?


templabel = aline.substr(0, aline.find_first_of('\t')); //Extract the label to temporary variable

How do you know aline contains a '\t' and what happens if it doesn't?
The files, assembly programs, are all tab-delimited. But, maybe, I can cause this program to simulate throwing a syntax error if not tab-delimited! By the way, it still freezes, even if I take your suggestion! In fact, if I create other program and throw this code in it, I get:
This application has requested the Runtime to terminate it in an unusual way. Please contact the application's support team for more instruction.
Am I going to have to make the string a c_str() and then check it???
Last edited on
Pages: 12