Solved

Segmentation fault in strcmp()

Posted on 2000-05-07
48
1,158 Views
Last Modified: 2008-03-17
I've been fighting this for nearly a week now and have had enough.  I need help - and FAST.

I'm trying to read from an serial infrared remote in Linux (using LIRC) and then compare the result against expected possible results.

Here is the code in question:

while(1) {
      //Read keypress
     keypress = CheckKeypress(irsd, *inout_fds);
    //strcpy(keypress, CheckKeypress(irsd, *inout_fds));
   //return 1;
 
            //Check for menu traverse (Down, Up, and Enter to select)
      if ((strcmp(keypress, "ChDown")) && (AutoSelect < 5))
         //if ((keypress = "ChDown") && (AutoSelect < 5))
            AutoSelect = AutoSelect+ 1;
      else if (strcmp(keypress, "ChUp"))  {
            if (AutoSelect > 1)
                  AutoSelect = AutoSelect- 1;
      }

---end of calling code  (there are more ifs for more buttons, but it crashes at the first strcmp call---  
You can see where I've commented out code that I've tried - none of it worked.  Although the return 1 did work (and the program continued on)..  so the problem must be in the strcmp function.

Here's the CheckKeypress Function:

//Remote button press check
char *CheckKeypress(int irsd, fd_set inout_fds)
{
  int i;
  IR irdata;
  IRmap keymap[IRKEYS];
  struct timeval timeout;
 
  timeout.tv_sec = 0;
  timeout.tv_usec = 0;
 
  FD_ZERO(&inout_fds);
  FD_SET(irsd,&inout_fds);
  i=select(16,&inout_fds,(fd_set *) 0, (fd_set *) 0, 0);

  if (FD_ISSET(irsd,&inout_fds)) {
    i = read(irsd, irdata.buf, sizeof(irdata.buf));
    ir_parsebuf(&irdata);
    return irdata.key;
  }
  else
    return NULL;
}

The function above looks good to me, but I'm wondering if it wouldn't be returning some bad data or something along the way.  I can't do a printf("%c", keypress) because there is no data shown...  so I have no way of knowing what is actually held in keypress.

PLEASE help - I have less than 24 hours to get this working.
0
Comment
Question by:Joe Cool
  • 18
  • 15
  • 6
  • +5
48 Comments
 
LVL 32

Expert Comment

by:jhance
ID: 2786351
You CheckKeypress() function can return NULL.  If (and when) it does so, your programs call to strcmp in:

if ((strcmp(keypress, "ChDown")) && (AutoSelect < 5))

_WILL_ crash.  You must not call strcmp with a NULL in either value.

Add a test for a NULL return after calling CheckKeypress().

0
 

Author Comment

by:Joe Cool
ID: 2786529
Well, I tried putting

if (keypress != NULL) {

}

around the outside of all the "if (strcmp(keypress, buttonname))" lines, but to no avail.  The program would wait for me to press a button, and then after I did, it crashed.  My only guess is that it has something to do with what is getting returned in irdata.key

Just for reference, here is the struct declaration for irdata:

typedef struct {
  char buf[128];         /*input */
  unsigned int  code;    /*returned*/
  unsigned int  serial ; /*returned*/
  char key[64];          /*returned*/
  char remote[64];       /*returned*/
  char rxkey;            /*returned. a key from rxkeys.h */
} IR;

The only one that matters to me is the key because that is text for the particular button.

Here is gdb's output (slightly parsed for space) when I run gdb ./dbproject

(gdb) r
Starting program: /player/./dbproject

Program received signal SIGSEGV, Segmentation fault.
0x400e2720 in strcmp () from /lib/libc.so.6
(gdb) backtrace
#0  0x400e2720 in strcmp () from /lib/libc.so.6
#1  0xbffffb0c in ?? ()
#2  0x804e006 in CheckKeypress (irsd=5, inout_fds={__fds_bits = {32,
        0 <repeats 31 times>}}) at project.cpp:580

I really know of nothing else to try.  Hopefully someone has a fix.
0
 
LVL 15

Expert Comment

by:NickRepin
ID: 2786593
char *CheckKeypress(int irsd, fd_set inout_fds)
{
  IR irdata;
  ...
  return irdata.key;
  ...
}


You are trying to return the STACK variable. It is destroyed once your function returns. And you may get an invalid pointer.

Try

   static IR irdata;

 
0
 
LVL 15

Expert Comment

by:NickRepin
ID: 2786599
Sorry, you return CHAR value.

typedef struct {
  ...
  char rxkey;
} IR;

But anyway, the type of your func is CHAR*.

Probably, it must be

typedef struct {
  ...
  char * rxkey;
} IR;


0
 
LVL 15

Expert Comment

by:NickRepin
ID: 2786602
Sorry again, my first answer is correct.

Use static declaration :

   static IR irdata;

 

0
 

Author Comment

by:Joe Cool
ID: 2786619
I'll give it a shot...    but should I declare that static in the main program and pass that variable to all my functions that use CheckKeypress?
0
 

Author Comment

by:Joe Cool
ID: 2786665
Well, I tried changing it as it was already declared to static, and I also tried commenting that out and adding the  static IR irdata;  line above main().

Unfortunately, neither worked.  There might be something with the char vs. char* part, though.  The thing is, there is a function that I didn't write that uses the structure and I'm trying to maintain it as is and change my own code to work with it.  So changing the return type of CheckKeypress would be better than changing any of the already done structures.

Hopefully, this can get resolved soon - I'm sure there are other unrelated bugs that I've got to get worked out before tomorrow.
0
 
LVL 6

Expert Comment

by:graham_k
ID: 2787033
hmm, did you do the obvious & step through it, one statement at a time, in a debugger?

I would suugest compiling with the -g option, then using xxgdb to find out exactly where it's blowing up, rather than trying to guess it by code reading.

btw, there's nothing wrong with passing a NULL to strcmp()

btw2, if you're uncomfortable with passing a return value of char* from a fn, just make it a parameter

e.g instead of

char *a = my_fn();

use

char a[99];  my_fn(a);
and have a() manipulate its parameter.
0
 

Author Comment

by:Joe Cool
ID: 2787146
Adjusted points from 500 to 750
0
 

Author Comment

by:Joe Cool
ID: 2787147
Well, I've messed with gdb some  (as you can see above), but I tried out xxgdb and nothing seemed to really work (probably something I'm doing wrong)... so it's back to gdb for now.

Ok...  but that doesn't affect the fact strcmp() is what is crashing in my code.  There has to be something being accessed that it doesn't like.

I don't really care about return values vs. parameters as long as the program works.

I'll be the first to admit I'm a linux programming newbie, but I've got to grow up fast.  I'm down to 12 hours left before this must be working.
0
 
LVL 15

Expert Comment

by:NickRepin
ID: 2787329
There may be several problems with your code.

1) As jhance said, you have to handle the NULL returned by CheckKeypress.

2) You have to use at least the "static" for irdata (or any other solution).
The following code is incorrect anyway:

char *CheckKeypress()
{
  IR irdata;
  ...
  return irdata.key;
}


3) The problem may be in the     ir_parsebuf().

It's possible that there is no terminating zero in the irdata.key.
Try this:

{ ...

  static IR irdata;

   ...
   memset(irdata.key,0,sizeof(irdata.key));
  ir_parsebuf(&irdata);
  return irdata.key;
  ...
}
0
 

Author Comment

by:Joe Cool
ID: 2788718
Well, for #1, I'm putting an
if (keypress != NULL) {
  ...  //all those ifs to check particular buttons
}

around the if / else if code... so there shouldn't be any calls to strcmp unless keypress is something other than NULL.

Regarding #2, I'm still not sure whether to put the declaration above main or in the CheckKeypress function.  It makes more sense to me above main, but I've tried it in both (and no go)

I tried #3 and detected no change.
0
 

Author Comment

by:Joe Cool
ID: 2788848
Also - if I compile this test code and include the ir.h file (for the ir_init() and ir_parsebuf() functions, it works perfectly.  I've got to be doing some stupid mistake...  but what?

//---test IR code---
#include <sys/time.h>
#include <sys/types.h>
#include <unistd.h>

int
ir_main()
{
  int irsd,i;
  fd_set  inout_fds;
  IR irdata;

  irsd=ir_init();
  ir_loadkeys();
 
  for(;;) {
    FD_ZERO(&inout_fds);
    FD_SET(irsd,&inout_fds);
    i=select(16,&inout_fds,(fd_set *) 0, (fd_set *) 0, 0);

    if (FD_ISSET(irsd,&inout_fds)) {
      i = read(irsd, irdata.buf, sizeof(irdata.buf));
      ir_parsebuf(&irdata);
      printf("key:%s,serial:%d, rxkey:%c\n",
        irdata.key,irdata.serial,irdata.rxkey);
    }
  }
}
0
 

Expert Comment

by:vhawargi
ID: 2789039
i dont kniow much about unix/linux..
but i tried to reproduce your problem using weird inputs to strcmp, on NT.
It DOES crash when the pointer (here, the first parameter of the strcmp) is NULL. (not "", but 0). try returning "" in the keypress function for the else part (and not NULL).

all teh best :-)
0
 
LVL 4

Expert Comment

by:mandhjo
ID: 2789557
Is the key attribute of the IR struct the one you want to be checking?  buf looks interesting.

Someone earlier asked if you have stepped through the code a line at a time...When you do, what do the variables look like right before the call to strcmp?  What does the entire IR struct look like before the CheckKeypress function returns?

Does this only happen in release mode?

0
 
LVL 4

Expert Comment

by:mandhjo
ID: 2789561
What is the value of AutoSelect when the program crashes?  
0
 
LVL 22

Expert Comment

by:CJ_S
ID: 2789836
exactly that's what I just wanted to say when reading through this.......

It might be that it will never get into one of those if statements because of the starting value from AutoSelect....

use an else-statement to see whether you get in there.......printf("blah??")
0
 
LVL 1

Expert Comment

by:BSoeters
ID: 2790398
May sound like a shot in the dark, but try to allocate a char buffer, in which you put "ChUp", in the test function you wrote, the one that worked:

char szChUp[] = "ChUp";

Then, try to do a

if (!strcmp(irdata.key, szChUp))
   printf ("irdata.key is equal to szChUp\n");
else
   printf ("irdata.key is not equal to szChUp\n");

or, possibly, to be absolutely certain:

if (!strcmp(irdata.key, &(szChUp[0])))
   printf ("irdata.key is equal to szChUp\n");
else
   printf ("irdata.key is not equal to szChUp\n");


If this works correctly, use solely character buffers to do your strcmp()'s in the non-working version, in the same manner as in one of the previous two examples that worked.


Also, irdata must eventually really be declared static, in the version that does not work in order to give it even a slight possibilty to work.

So put this in your non-working version, in the CheckKeypress() function:

static struct IR irdata;


Another shot in the dark, instead of returning irdata.key, try to return &(irdata.key) or &(irdata.key[0]).



Testing for NULL's when doing strcmp() is always safer than not testing for NULL's, but, as far as I know, strcmp() does not have problems when comparing strings to NULL pointers.




As a test, try to make your non-working version work by doing the same thing that you did with the working test version.

Try to merge the CheckKeypress() function into the calling function. If that does not result in working code, you know nothing's wrong with the setup of the original non-working version.



Please, give it a try... I wish you all the best in advance.
0
 
LVL 6

Expert Comment

by:graham_k
ID: 2790555
if you've messed with gdb, then you know for sure that it's blowing up on strcmp(). So, what are the values of the two strings?

If you still don't know, then add a few printf("**%s**\n", ...)
maybe a strlen() too. I like the idea of the string maybe not being null terminated (in which case, user strncmp() which compares two strings for n chars, not until end of string).

This can not be so, complicated, if you're sure that it's the strcmp()
0
 
LVL 1

Expert Comment

by:BSoeters
ID: 2790564
Graham k, how do you explain the following if the string would not be properly terminated ?

BTW, strncmp() does stop at the end of either of the two strings (the smallest), or after comparing n characters of both strings, whichever comes first.

From: NickRepin
Date: Monday, May 08 2000 - 08:30AM CEST  

3) The problem may be in the     ir_parsebuf().

It's possible that there is no terminating zero in the irdata.key.
Try this:

{ ...

  static IR irdata;

   ...
   memset(irdata.key,0,sizeof(irdata.key));
  ir_parsebuf(&irdata);
  return irdata.key;
  ...
}

From: Joe Cool
Date: Monday, May 08 2000 - 04:40PM CEST  

I tried #3 and detected no change.


0
 

Author Comment

by:Joe Cool
ID: 2790740
DOH..  I feel so stupid... words can't describe.

The strcmp failure in question is in ir_parsebuf()   I finally figured out how to step in gdb and now we are getting somewhere.

Here's the gdb output:
588         ir_parsebuf(&irdata);
(gdb) step
ir_parsebuf__FP2IR (irs=0x810f980) at ir.c:51
51        int i=0;
Current language:  auto; currently c
(gdb) step
52        sscanf(irs->buf,"%x%x%s%s",&(irs->code),&irs->serial,irs->key,irs->remote);
(gdb) step
54        while ((strcmp(keymap[i].irkey,irs->key)) && i<(IRKEYS-1)) i++;
(gdb) step
 
Program received signal SIGSEGV, Segmentation fault.
0x400e2720 in strcmp () from /lib/libc.so.6


So there you have it..  here's the ir_parsebuf() function in its entirety:

void
ir_parsebuf(IR * irs) {
  int i=0;
  sscanf(irs->buf,"%x%x%s%s",&(irs->code),&irs->serial,irs->key,irs->remote);

  while ((strcmp(keymap[i].irkey,irs->key)) && i<(IRKEYS-1)) i++;

  if ((!strcmp(keymap[i].irkey,irs->key)))
    irs->rxkey=keymap[i].rxkey;
  else
    irs->rxkey=0;
}

Sorry to lead you all down the wrong road there initially...   (whoops :-/ )
0
 
LVL 1

Expert Comment

by:BSoeters
ID: 2790753
change this line:

while ((strcmp(keymap[i].irkey,irs->key)) && i<(IRKEYS-1)) i++;

to this:

while (i<(IRKEYS-1) && (strcmp(keymap[i].irkey,irs->key))) i++;

and this line:

if ((!strcmp(keymap[i].irkey,irs->key)))

to this:

if (i < (IRKEYS-1) && (!strcmp(keymap[i].irkey,irs->key)))

and you're done...


(BTW, are you sure it's i<(IRKEYS-1) and not i<IRKEYS that you'd want to test ? You could be missing the last index you know...
0
 
LVL 1

Expert Comment

by:BSoeters
ID: 2790765
) (mmm, had to close another parantheses on that BTW remark)

You could also assert that the incoming "irs" pointer does not point to NULL, as that would lay ground for another sortlike question :)
0
 

Author Comment

by:Joe Cool
ID: 2790774
DOH..  I feel so stupid... words can't describe.

The strcmp failure in question is in ir_parsebuf()   I finally figured out how to step in gdb and now we are getting somewhere.

Here's the gdb output:
588         ir_parsebuf(&irdata);
(gdb) step
ir_parsebuf__FP2IR (irs=0x810f980) at ir.c:51
51        int i=0;
Current language:  auto; currently c
(gdb) step
52        sscanf(irs->buf,"%x%x%s%s",&(irs->code),&irs->serial,irs->key,irs->remote);
(gdb) step
54        while ((strcmp(keymap[i].irkey,irs->key)) && i<(IRKEYS-1)) i++;
(gdb) step
 
Program received signal SIGSEGV, Segmentation fault.
0x400e2720 in strcmp () from /lib/libc.so.6


So there you have it..  here's the ir_parsebuf() function in its entirety:

void
ir_parsebuf(IR * irs) {
  int i=0;
  sscanf(irs->buf,"%x%x%s%s",&(irs->code),&irs->serial,irs->key,irs->remote);

  while ((strcmp(keymap[i].irkey,irs->key)) && i<(IRKEYS-1)) i++;

  if ((!strcmp(keymap[i].irkey,irs->key)))
    irs->rxkey=keymap[i].rxkey;
  else
    irs->rxkey=0;
}

Sorry to lead you all down the wrong road there initially...   (whoops :-/ )
0
Better Security Awareness With Threat Intelligence

See how one of the leading financial services organizations uses Recorded Future as part of a holistic threat intelligence program to promote security awareness and proactively and efficiently identify threats.

 

Author Comment

by:Joe Cool
ID: 2790779
sorry about the double post there...  I'll give your suggestion a shot

Actually, I'm not totally sure what I want to test.  I didn't write the ir_parsebuf() function and most of the CheckKeypress() function  (pulled much of the code out of another main function and tried to functionize it.
0
 
LVL 1

Expert Comment

by:BSoeters
ID: 2790802
(Another BTW, "keymap" is valid and accessible isn't it ? And "keymap[i].irkey" is also valid (given i < IRKEYS) and resolves to a pointer doesn't it ?)

I still think that you're not seeing the last index, but perhaps you could test that either by stepping through using the debugger, or by filling keymap with sensible values and examining the return value of ir_parsebuffer().
0
 

Author Comment

by:Joe Cool
ID: 2790814
Well, what I don't understand is why the test code above would work, but not what I morphed the test code into  (the CheckKeypress() function)

I'm gonna have to grab an hour of sleep - my brain isn't hardly functioning...  I'll check out some implementations and possible then.

I did try your fix and it still crashed.  when I tried print irs->key in gdb, it said irs wasn't defined in this scope... which seems weird.

But anyways....  I'm rambling
0
 
LVL 1

Expert Comment

by:BSoeters
ID: 2790894
The following two lines are probably causing the separate test to succeed, while letting your main project fail.

  irsd=ir_init();
  ir_loadkeys();

ir_parsebuffer() depends on the keymap array, which probably is global. That array could be initialized by ir_init() or ir_loadkeys() without us knowing it (just like the ir_parsebuffer() being faulty without us knowing it).

If you add the two lines to your normal project (in a slightly modified way if you will), does it still crash ?

I wonder where your main project crashes, after introducing my "fixes". Can you elaborate on that matter ? I certainly do not hope that ir_parsebuffer() is still crashing, except for maybe the possible initialization matter of the keymap array...

Well, above all, I hope you get a good night sleep :)
0
 

Author Comment

by:Joe Cool
ID: 2790922
Just needed a nap.

ir_parsebuffer() still crashes at the exact same place.  I'll have to mess with some of the variable stuff in parsebuffer.
0
 
LVL 1

Expert Comment

by:BSoeters
ID: 2790948
The irs pointer is not null, is it ?

From a previous comment of mine, ..."(Another BTW, "keymap" is valid and accessible isn't it ? And "keymap[i].irkey" is also valid (given i < IRKEYS) and resolves to a pointer doesn't it ?)"...

So, we're still crashing at the strcmp() ?

If it still is the strcmp() in the while statement, then either "keymap[i].irkey" is not accessible (readable) or "irs->key" is not accessible (readable). And perhaps "keymap[i]" is not accessible.

I don't know what type of array "keymap" is.
0
 
LVL 1

Expert Comment

by:BSoeters
ID: 2790998
The irs pointer is not null, is it ?

From a previous comment of mine, ..."(Another BTW, "keymap" is valid and accessible isn't it ? And "keymap[i].irkey" is also valid (given i < IRKEYS) and resolves to a pointer doesn't it ?)"...

So, we're still crashing at the strcmp() ?

If it still is the strcmp() in the while statement, then either "keymap[i].irkey" is not accessible (readable) or "irs->key" is not accessible (readable). And perhaps "keymap[i]" is not accessible.

I don't know what type of array "keymap" is.
0
 
LVL 1

Expert Comment

by:BSoeters
ID: 2791009
Sorry for the double post :(
0
 
LVL 4

Expert Comment

by:mandhjo
ID: 2791046
Your code:

while ((strcmp(keymap[i].irkey,irs->key)) && i<(IRKEYS-1)) i++;

should be changed to check the contraint on i *first*, ie,

while (i<(IRKEYS-1) && (strcmp(keymap[i].irkey,irs->key))) i++;

The way you have it, i will become greater than (IRKEYS - 1) and you will access keymap[i] BEFORE you realize that it is out of bounds.  By checking that i is less than (IRKEYS - 1) first, you will kick out of the while loop and not do the strcmp on an invalid memory address.

Please try this change and report on your results.

Mark

0
 
LVL 4

Expert Comment

by:mandhjo
ID: 2791054
Further comments:

An AND condition, &&, will check every condition until one of them is false.  As soon as one of them is false, it will stop checking the other conditions and return FALSE.

If you have this code:

char szStuff[2];
szStuff[0] = 'M';
szStuff[0] = 'J';

int n = 0;

while ((szStuff[n] != 'X') && (n < 2))
  n++;

The execution would look like this:
n is 0
'M' is not 'X' and 0 < 2 so increment n
n is 1
'J' is not 'X' and 1 < 2 so increment n
n is 2
KABLOOWIE

The next code to execute will be to compare szStuff[2] to 'X'.
szStuff only has two entries, so accessing the third one is an access violation (or segmentation fault).

Now change the code:

char szStuff[2];
szStuff[0] = 'M';
szStuff[0] = 'J';

int n = 0;

while ((n < 2) && (szStuff[n] != 'X'))
  n++;

Execution:
n is 0
0 < 2 and 'M' is not 'X' so increment n
n is 1
1 < 2 and 'J' is not 'X' so increment n
n is 2
2 < 2 -- this fails, so the second condition is never executed and you don't blow up

Did this fix your problem?
0
 
LVL 1

Expert Comment

by:BSoeters
ID: 2792416
Joe Cool, how are you doing ? Did you try to debug ir_parsebuffer() ?
0
 

Author Comment

by:Joe Cool
ID: 2792485
Sorry... had to put my attention on some real work rather than this code last night.  I'll get on it now, and see what I come up with.

Right after it crashed, I looked at keymap, and it seemed filled with nulls (according to my memory).  irs seemed non-existant.

But anyway, I'll get work on it and report my findings.
0
 
LVL 1

Expert Comment

by:BSoeters
ID: 2792778
That's fine :)

The likelihood of keymap being wrong is fairly strong.

After fixing keymap, keymap[i] must be looked at.
After it is assured that keymap[i] is right, keymap[i].irkey must be looked at.
0
 
LVL 4

Expert Comment

by:mandhjo
ID: 2792827
My example above should have the following code:

char szStuff[2];
szStuff[0] = 'M';
szStuff[1] = 'J'; // not the change from 0 to 1 in the index

for each example.  I'm sure you would have all figured that out by now.

0
 

Author Comment

by:Joe Cool
ID: 2793111
I decided to toast all the code in ir_parsebuf() except the scanf line.  The reason for this is that I don't really care about the rxkey code -- that only mattered to the guy that wrote the code... I don't use rxkey for anything (nor do I plan to :)).

And now it works fine..  ir_parsebuf() puts "One" /000 (60 times) in irdata.key in CheckKeypress.  However, now strcpy(keypress, CheckKeypress()) is crashing...  I'm going to try an '=' instead and see if that works.

I still don't understand why the code would work perfectly in the test program, but not in my real project.  If someone wouldn't mind explaining that, I'd appreciate it.

And thanks mandhjo for your explanation of how the while( ... && ...) would have worked.  That just hadn't clicked--I've got to think like a computer. :)
0
 
LVL 1

Expert Comment

by:BSoeters
ID: 2793152
For the &&, that's what I meant when I posted the "fixes", after you determined that ir_parsebuffer() was the problem. But I did not include the reasons, but mandhjo's explanation filled that gap.

...."However, now strcpy(keypress, CheckKeypress()) is crashing...  I'm going to try an '=' instead and see if that works."...

Could you post some code concerning this ? If keypress is a char*, and keypress isn't pointing to a valid memory block (i.e. uninitialized or set to NULL), this would be crashing.

The assignment operator would work, if keypress is a char* and you included the static keyword before irdata in your CheckKeypress() function. If you did not include the static keyword, this would be crashing.

If keypress is a character array, and it's large enough to hold the character string of which the address is returned by CheckKeypress() (plus a terminating '\0'), the strcpy() ought to work, as long as you included the static keyword, as mentioned above.
0
 

Author Comment

by:Joe Cool
ID: 2793227
sorry...  mornings are tougher than nights.  :)

code:
keypress = CheckKeypress(... parameters ...);
rather than
strcpy(keypress, CheckKeypress(...params...));

the first line works... the second crashes.

For some reason, I've managed to get in an infinite loop in the button compare part of the main project (and it's not checking all of the buttons).
while(1)
  if (strcmp(keypress, "Button"))
    return 1;
  else if (strcmp(keypress, "Enter"))
    return 2;
  ....etc...

However, I think the worst is over...  now I have to fix that and get some other stuff together.

Now I have to figure out how to dole out these points :(  Tell ya what - if I really do have the button checks fixed, the first person to give a solid explanation of why the system would crash with my project vs. the test code (worked fine), will get a B.  :)  At least then, this could be useful to people in the future...
0
 
LVL 1

Expert Comment

by:BSoeters
ID: 2793258
Button checks are wrong.

Change (for example):

if (strcmp(keypress, "Enter"))

to

if (!strcmp(keypress, "Enter"))

strcmp() returns 0 for equal strings.
0
 
LVL 1

Accepted Solution

by:
BSoeters earned 750 total points
ID: 2793268
And about that crash with the whole project, and not with your test project, I believe I gave that a shot earlier...

I think my explanation is plausible - the comment with

...."The following two lines are probably causing the separate test to succeed, while letting your main project fail.

  irsd=ir_init();
  ir_loadkeys();

ir_parsebuffer() depends on the keymap array, which probably is global. That array could be initialized by ir_init() or ir_loadkeys() without us knowing it (just like the ir_parsebuffer() being faulty without us knowing it).

"...

in it :).
0
 
LVL 4

Expert Comment

by:mandhjo
ID: 2793292
Regarding this:

code:
keypress = CheckKeypress(... parameters ...);
rather than
strcpy(keypress, CheckKeypress(...params...));


How is keypress defined?  Is it a char *?

If it is a char *, ie,
char * keypress;
then the code will definitely blow up.

strcpy assumes that the destination paramter has enough space allocated for the target data.  When you have a pointer to a character and you intend to copy data into that memory location, you better allocate space for it.

This approach:
keypress = CheckKeypress(... parameters ...);

may not blow up, but it isn't really valid either (necessarily).  It all depends on your implementation of Checkkeypress.  Someone several comments back mentioned that if you declare a variable on the stack in a function, you can't return a pointer to it FROM the function because by the time you access the pointer, the stack will be blown away and the pointer will be invalid.

The infinite loop question:
Does the code above have a simple "else" that will return a value?

While (1)
{
  if (condition1)
    return 1;
  else if (condition2)
    return 2;
}

will go on forever if neither condition1 nor condition2 ever become true.  You need to have an "else" or default return value, eg,

While (1)
{
  if (condition1)
    return 1;
  else if (condition2)
    return 2;
  else
    return -1;
}

Why do you have a while(1) anyway?  I assume there is other processing in that while loop that you didn't show us.
0
 

Author Comment

by:Joe Cool
ID: 2793306
Hmm.. although the first line of code in main() on my project is
irsd = ir_init();

I never ran ir_loadkeys();  I had thought about making keymap global, but I then realized I really didn't need it.

Thanks for the button check thing... I realized that and now all my checks are
if (strcmp(keypress, "button) == 0)
which I think is a bit easier to read than with the ! in there  (makes it look like  if keypress is not button )

And yeah - I couldn't just send all of my code - this is by far the largest program I've ever written... over 1500 lines of code at the moment.

I thank you all... and the points go to BSoeters for not only being the fastest with fixes, but also having the most posts/staying power (double post notwithstanding ;) ).  Thanks.
0
 

Author Comment

by:Joe Cool
ID: 2793312
BTW, I used while(1) because I want the code to loop through until a selection has been made... and I didn't want 15 different checks in the loop declaration.
0
 
LVL 1

Expert Comment

by:BSoeters
ID: 2793341
Thank you very much, Joe Cool.

I hope this kind of trouble will stay out of your way in the future :).
0
 

Author Comment

by:Joe Cool
ID: 2793368
no prob... thank you.

Me too. :)
0

Featured Post

IT, Stop Being Called Into Every Meeting

Highfive is so simple that setting up every meeting room takes just minutes and every employee will be able to start or join a call from any room with ease. Never be called into a meeting just to get it started again. This is how video conferencing should work!

Join & Write a Comment

Written by John Humphreys C++ Threading and the POSIX Library This article will cover the basic information that you need to know in order to make use of the POSIX threading library available for C and C++ on UNIX and most Linux systems.   [s…
What is C++ STL?: STL stands for Standard Template Library and is a part of standard C++ libraries. It contains many useful data structures (containers) and algorithms, which can spare you a lot of the time. Today we will look at the STL Vector. …
The goal of the video will be to teach the user the difference and consequence of passing data by value vs passing data by reference in C++. An example of passing data by value as well as an example of passing data by reference will be be given. Bot…
The viewer will learn how to pass data into a function in C++. This is one step further in using functions. Instead of only printing text onto the console, the function will be able to perform calculations with argumentents given by the user.

744 members asked questions and received personalized solutions in the past 7 days.

Join the community of 500,000 technology professionals and ask your questions.

Join & Ask a Question

Need Help in Real-Time?

Connect with top rated Experts

13 Experts available now in Live!

Get 1:1 Help Now