Running programs - few stupid mistakes

Hello, back again from a while of non-programming... And having problems with the following project.
I'm writing a program that aims to take in characters, any number, hit by the user, and run programs according to what was entered. I want to be able to run this program, hit "m", and get my WinAmp run, or hit "mo" and get my Winamp and Opera run. However, I'm having a lot of problems, and I'll probably kick myself when I'm told what I'm doing wrong. I'd be grateful if people could take a look. Thanks.

Code:

#include <iostream.h>
#include <windows.h>
#include <conio.c>

int main()
{
    MessageBox(NULL, "Hit OK, type the passkeys\nand hit ENTER.", "Run", MB_OK);
    char a[7];
    char* pA = a;
    cin >> a;
    char* b = "m";
    char* c = "o";
    char* d = "t";
    char* e = "i";
    char* f = "k";
    char* g = "w";
    bool music = FALSE;
    bool opera = FALSE;
    bool trillian = FALSE;
    bool imesh = FALSE;
    bool kazaa = FALSE;
    bool winmx = FALSE;
    for(int i = 0; i < 8; i++)
    {
        if(strcmp(b, &pA[i]) == 1)
        {
                music = TRUE;
        }
        if(strcmp(c, &pA[i]) == 1)
        {
                opera = TRUE;
        }
        if(strcmp(d, &pA[i]) == 1)
        {
                trillian = TRUE;
        }
        if(strcmp(e, &pA[i]) == 1)
        {
                imesh = TRUE;
        }
        if(strcmp(f, &pA[i]) == 1)
        {       
                kazaa = TRUE;
        }
        if(strcmp(g, &pA[i]) == 1)
        {
                winmx = TRUE;
        }
    }
    
    if(music == 1) system("C:\\Documents and Settings\\All Users.WINDOWS\\Documents\\Quent\\Music.m3u");
    if(opera == 1) system("C:\\Program Files\\Opera7\\Opera.exe");
    if(trillian == 1) system("C:\\Program Files\\Trillian\\trillian.exe");
    if(imesh == 1) system("C:\\Program Files\\iMesh\\Client\\iMeshClient.exe");
//    if(kazaa == 1) 
    if(winmx == 1) system("C:\\Program Files\\WinMX\\WinMX.exe");
    MessageBox(NULL, "End of Program.", "Run", MB_OK);
    return 0;
}

Don't mind the bad presentation, I had to declare variables when the original way I coded it didn't work, now it compiles but doesn't run properly. Thanks for any help.

MessageBox(NULL, "Hit OK, type the passkeys\nand hit ENTER.", "Run", MB_OK);

That's Windows API right? If you want to make a Windows Program you have to change function main to function WinMain, (which has four parameters.) Then, you have to have your compiler compile it as a Windows program.

No vulcan, you can actually use windows functions in a console program, because (surprise surprise!) console programs are actually in windows anyways.

Korhedron:
First of all, you should use false instead of FALSE, if you're dealing with bool instead of BOOL, and likewise with true instead of TRUE.

Then, at the end, instead of "if(winmx == 1)", use either "if(winmx == true)", or better IMO, "if(winmx)".

Technically, winmx (and the other variables like it) should only be storing 'true' or 'false', because that's what a bool is. Second, true is only defined to be non-zero, and false is defined to be zero. So what happens if TRUE (the Microsoft define) happens to be 345 or 23 or 999? Your if's will fail in that case (although I doubt it is). Likewise with true.

Now for the real problems:

Code:

    char a[7];
    char* pA = a;   //Don't need pA.
    cin >> a;

Code:

if(strcmp(b, &pA[i]) == 1)

1. Like above, compare with a instead of pA. An array is actually a pointer.
2. strcmp() will return 0 if strings are the same, non-zero for different.
3. &pA[i] isn't just one character, it's everything from pA[i] to the NULL character at the end.

This is what you should do instead:

Code:

if(a[i] == 'm')

And so on, for the other variables. Also, just as a note, you don't need all the opening/closing braces if you only have 1 line in an if/for/while

Thank you Hunter2, your input was very helpful and I could get my program to work. However, I still have a few problems. Here is the updated code.

Code:

#include <iostream.h>
#include <windows.h>
#include <conio.c>

// GLOBAL VARS
    bool music = false;
    bool opera = false;
    bool trillian = false;
    bool imesh = false;
    bool kazaa = false;
    bool winmx = false;


int main()
{
    MessageBox(NULL, "Hit OK, type the passkeys\nand hit ENTER.", "RoverRun", MB_OK);
    char a[7];
    cin >> a;



    for(int i = 0; i < 8; i++)
    {
        if(a[i] == 'm') music = true;
        if(a[i] == 'o') opera = true;
        if(a[i] == 't') trillian = true;
        if(a[i] == 'i') imesh = true;
        if(a[i] == 'k') kazaa = true;
        if(a[i] == 'w') winmx = true;
    }
    
    if(music)
    {
        system("cd..");
        system("cd..");
        system("cd Documents and Settings\\All Users.WINDOWS\\Documents\\Quent");
        system("Music.m3u");
    }
    if(opera)
    {
        system("cd..");
        system("cd..");
        system("cd Program Files\\Opera7");
        system("Opera.exe");
    }
    if(trillian)
    {
        system("cd..");
        system("cd..");
        system("cd Program Files\\Trillian");
        system("trillian.exe");
    }
    if(imesh)
    {
        system("cd..");
        system("cd..");
        system("cd Program Files\\iMesh\\Client");
        system("iMeshClient.exe");
    }
    if(kazaa)
    {
        system("cd..");
        system("cd..");
        system("cd Program Files\\Kazaa");
        system("klrun.exe");
    }
    if(winmx)
    {
        system("cd..");
        system("cd..");
        system("cd Program Files\\WinMX");
        system("WinMX.exe");
    }
    
/*    char mbox[];
    int j = 0;
    if(music) { mbox[j] = "Music Playlist launched.\n"; j += 25; }
    if(opera) { mbox[j] = "Opera Browser launched.\n"; j += 25; }
    if(trillian) { mbox[j] = "Trillian Chat Client launched.\n"; j += 25; }
    if(imesh) { mbox[j] = "iMesh P2P Client launched.\n"; j += 25; }
    if(kazaa) { mbox[j] = "KaZaA P2P Client launched.\n"; j += 25; }
    if(winmx) { mbox[j] = "WinMX P2P Client launched.\n"; j += 25; }
*/    
    MessageBox(NULL, "Program terminated.", "RoverRun", MB_OK);
    return 0;
}

My Music runs without a problem. However, iMesh, Kazaa, Trillian and WinMX all give me "The system cannot find the path specified. 'iMeshClient.exe' is not recognised as an internal or external command, operable program or batch file."
However, if I follow the program manually ( Run cmd, "cd..", etc. ) I can run iMesh, and the other programs, with no problems.

Finally, I cannot test if this program works with my multiple commands ( therefore if char a = "tm" for Trillian and Music ) due to the fact that only the "m" command works.

Any tips would be great Thanks.

Still a couple of problems with the code:

You declare char a[7], but don't initialize it.

Then, after reading one or more characters with cin, you look at eight characters.

If you want keep the same program structure, initialize the array (to all 0, for example), then change your loop to go through 7 chars, not 8.

As far as why your stuff doesn't work, I'm not sure about the system() calls. After a return from system(), the next call is still from the current environment (I think).

Try this: make a test case consisting of the following

Code:

    system("cd ..");
    system("dir");

Does it show the "dir" of the current directory or its parent?

Dave

system() calls aren't portable. You would be better of using CreateProcess(). It allows you to control the application you spawn better as well.

>system() calls aren't portable.
That won't work. Be more specific.

How will it not work? CreateProcess() can open any app as long as the path is specified. If you wanted me to go into a full description of the function, sorry I don't have the time.

>How will it not work?
Your explanation was too vague.

>CreateProcess() can open any app as long as the path is specified.
You'll notice that I tend to quote the most relevant part of a post that I'm commenting on. I have nothing against your suggestion of CreateProcess.

>If you wanted me to go into a full description of the function, sorry I don't have the time.
No, I wanted you to be more specific concerning your claim of system not being portable. The function itself is portable to any platform that supports C++, but the argument you pass to it may not be portable. You might not think that the distinction is important, but I do. Besides, why did you even bother mentioning portability when you were going to suggest a solution that was even less portable than a standard library function?

Ok sorry. I was under the impression that system was strictly windows. My mistake. And as for CreateProcess, yeah its less portable. I didn't bother looking that up.

>>You declare char a[7], but don't initialize it.
I don't see what's wrong with that. Who says you need to initialize an array to use it?

Hmm, not sure exactly the problem with "bad path", but I'll give a list of possible reasons I can think of:
-every call to system() is independent, and will reset the directory to the one the exe is in after it returns. So in other words, your cd..'s and cd somePath's are doing nothing

-system() calls might need the 8.3 filename format (8 chars followed by . and then a 3-char extension), so instead of your long "(...)Documents and Settings(...)" things, it might need "(...)docume~1(...)" or something. Again, not 100% sure. It's probably not the case.

-Instead of doing the cd.. stuff, try something like this:

Code:

system("C:\\Program Files\\Kazaa\\klrun.exe");
**or**
system("C:\\Progra~1\\Kazaa\\klrun.exe");

Hope this helps!

Well, of course you don't have to initialize an array before putting something into it, but the program reads the contents of all elements of the array; what if the user only entered one character (before hitting return). Then a[0] has his character --- what's in a[1], a[2], ... ?

Of course there is a real serious problem with getting the characters into the array, even if it is initialized to all zeros.

What if the user enters more than 7 characters?

I think I would rather see the program collect user input using something like the getline function (which can limit the number of characters to less than equal to a specified number). Then the loop could look at all of the characters that were entered and act accordingly.

Dave

Oops, overlooked that part about the program reading the whole array I was thinking about how cin automatically adds a NULL at the end of input...

But yeah, probably should make the loop more like
for(int i = 0; i < strlen(cmd); ++i)

And, yes, possibly use getline instead of >>. Although if you make a large enough array, say 128, it's highly unlikely that you're going to want to open that many programs, especially since there's only 6 to choose from. As long as you don't distribute the program to people whose only intentions are to break it, there should be no problem

Well, since it's your program, and I am sure that you wouldn't enter more than, say, 80 characters (actually, of course, lots fewer than that), you might think it's safe to define an array of length 80. But why would you do that?

I see no problem if you want to leave things pretty much as-is in order to see what's happening with your system() problems, but before you finish, you should test it with all kinds of inputs (suppose no characters were entered),
(suppose you doze off with your finger on a key, and it repeats more than 80 characters).

Actually, I feel that it is never too early to consider what would happen in case unexpected input patterns occur.

I know it seems uninportant for this program, but you should always try to make things bullet-proof; guard against unexpected input.

Good luck, and best regards!

Dave

Thank you all for your input, it was very interesting to see the possible causes of failure. It was my system calls that were stopping the program from running correctly, I was unaware that a new call to system was independant. To fix this, I had to create a new directory and place shortcuts to each file, and run these .lnk files. I couldn't run the programs from their installed locations, as I didn't manage to use system() with a space in the name of the directory ( cmd.exe interpreted up to the space as a command, as it should. Using the 8.3 format also did not work ). My current system calls are therefore as follows :

Code:

system("C:\\runshortcuts\\music.lnk");

Now the program will execute all the commands properly, and I'm now starting to test its faults ( as Dave Evans said, using too many or no commands, etc. ).

Again, thanks to everyone