473,323 Members | 1,560 Online
Bytes | Software Development & Data Engineering Community
Post Job

Home Posts Topics Members FAQ

Join Bytes to post your question to a community of 473,323 software developers and data experts.

strcmp() fooling me? :o

Hi,

I got a problem: I have a function that allocates new memory to hold another
element of my struct array if it's not existing already.
Expand|Select|Wrap|Line Numbers
  1. /** My Structures **/
  2. typedef struct {
  3. char BusID[6];
  4. int SeqNr;
  5. unsigned short Msg;
  6. } MsgStruct;
  7. typedef struct{
  8. unsigned char  app;
  9. unsigned char  pri;
  10. unsigned char  act;
  11. int            seq;
  12. char   id[6];
  13. }io_t;
  14. /* My function */
  15. int CheckStruct(MsgStruct **SumStruct_ptr, io_t *CurrentBus)
  16. {
  17. int i=0;
  18. if (StructCount==0){
  19. /* allocating memory to hold the 1st element */
  20. }
  21. /* loop thru all elements in the struct array and check if
  22. busID and seq# matches with an existing element
  23. if not, create new element */
  24. for (i=0; i < StructCount; i++){
  25. if((strcmp((*SumStruct_ptr)[i].BusID, CurrentBus->id)==0) &&
  26. ((*SumStruct_ptr)[i].SeqNr== CurrentBus->seq)){
  27. return i;
  28. } /*if we're at the end of the array and nothing matched -  extend
  29. memory*/
  30. else if( i == StructCount - 1){
  31. (*SumStruct_ptr) = realloc((*SumStruct_ptr),
  32. (StructCount+1)*sizeof(MsgStruct ));
  33. StructCount++;
  34. strncpy((*SumStruct_ptr)[i+1].BusID, CurrentBus->id, 6);
  35. (*SumStruct_ptr)[i+1].SeqNr=CurrentBus->seq;
  36. (*SumStruct_ptr)[i+1].Msg=0;
  37. prs_log(LOG_NOTICE,"***CheckStruct() - Added new element[%d]: BusID:%s
  38. seqNr:%d - Count: %d", StructCount-1, (*SumStruct_ptr)[i+1].BusID,
  39. (*SumStruct_ptr)[i+1].SeqNr, StructCount);
  40. return i+1;
  41. }
  42. }
  43. }
  44. MsgStruct *
  45. ClearElement(MsgStruct ** SumStruct_ptr,
  46. int         Position)
  47. {
  48. /* Check for invalid 'Position' value */
  49. prs_log(LOG_NOTICE,"***ClearElement() - Will remove element [%d]: BusID
  50. %s seqNr:%d - Count: %d", Position,(*SumStruct_ptr)[Position].BusID,
  51. (*SumStruct_ptr)[Position].SeqNr, StructCount);
  52. if ( Position>= StructCount )
  53. return (*SumStruct_ptr);
  54.  
  55. /* If 'Position' doesn't index the very last element move all the
  56. ones following it to a position with an index smaller by 1,
  57. thereby overwriting the element at 'Position' */
  58.  
  59. if ( Position != StructCount - 1 )
  60. memmove( SumStruct_ptr + Position, SumStruct_ptr + Position + 1,
  61. ( StructCount - Position - 1 ) * sizeof *SumStruct_ptr );
  62.  
  63. /* The last element isn't used anymore, so give back the memory
  64. used for it. */
  65. StructCount--;
  66. return (*SumStruct_ptr) = realloc( (*SumStruct_ptr), StructCount *
  67. sizeof *SumStruct_ptr );
  68.  
  69.  
And this is what happened (copy/paste from my syslog - where prs_log() is
writing to):
"CheckStruct() - Added new element[2]: BusID:T9998 seqNr:9 - Count: 3"
"ClearElement() - Will remove element [0]: BusID:T1111 seqNr:627 - Count: 2"
"CheckStruct() - Added new element[2]: BusID:T9998 seqNr:9 - Count: 3"
Now... Why did it create a second entry tor T9998, SEQ#9?
Shouldn't it have recognized the one that's there already? Held on position
1 after the element 0 has been cleared or do you see some mistake in my
clearing function? :o I get pretty stuck here...
Thanks for everybody's hints and suggestions!
Ron
--
weeks of software enineering safe hours of planing ;)
Jun 27 '08 #1
8 2116
Ron Eggler wrote:
[...]
MsgStruct *
ClearElement(MsgStruct ** SumStruct_ptr,
int Position)
{
[...]
if ( Position != StructCount - 1 )
memmove( SumStruct_ptr + Position, SumStruct_ptr + Position + 1,
( StructCount - Position - 1 ) * sizeof *SumStruct_ptr );
You've forgotten one level of indirection:

memmove( (*SumStruct_ptr) + Position,
(*SumStruct_ptr) + Position + 1,
(StructCount - Position - 1) * sizeof **SumStruct_ptr);

--
Er*********@sun.com
Jun 27 '08 #2
On Jun 5, 6:18 pm, Ron Eggler <unkn...@example.comwrote:
Thanks for everybody's hints and suggestions!
I recommend that you move *SumStruct_ptr into a local variable as
early as possible. This will make your code much more readable and the
bug in your code very obvious. Also, it seems quite weird that you
pass a pointer that is modified by address, and use another related
variable as a global variable. If you had a single struct containing
the pointer and the count, the code would be much cleaner and again,
the error would become obvious.

Jun 27 '08 #3
Another recommendation: Your combination of using strncpy to fill an
array of char and strcmp to compare the contents is dangerous. Either
your strings are limited to five characters and fit, then use strcpy.
Or they can be longer than five characters, then the call to strcmp is
dangerous and can give the wrong result.
Jun 27 '08 #4
Eric Sosman wrote:
Ron Eggler wrote:
>[...]
MsgStruct *
ClearElement(MsgStruct ** SumStruct_ptr,
int Position)
{
[...]
if ( Position != StructCount - 1 )
memmove( SumStruct_ptr + Position, SumStruct_ptr + Position + 1,
( StructCount - Position - 1 ) * sizeof *SumStruct_ptr
);

You've forgotten one level of indirection:

memmove( (*SumStruct_ptr) + Position,
(*SumStruct_ptr) + Position + 1,
(StructCount - Position - 1) * sizeof **SumStruct_ptr);
Exactly, great - thank you :)

--
weeks of software enineering safe hours of planing ;)
Jun 27 '08 #5
christian.bau wrote:
On Jun 5, 6:18 pm, Ron Eggler <unkn...@example.comwrote:
>Thanks for everybody's hints and suggestions!

I recommend that you move *SumStruct_ptr into a local variable as
early as possible. This will make your code much more readable and the
bug in your code very obvious. Also, it seems quite weird that you
pass a pointer that is modified by address, and use another related
variable as a global variable. If you had a single struct containing
the pointer and the count, the code would be much cleaner and again,
the error would become obvious.
Well the reason for me using a double pointer is, because i want to use the
array elements from my main loop and this specific struct variable (in the
array) should keep its value from time of allocation until it is destroyed
again in ClearElement().
Does this make more sense now?

--
weeks of software enineering safe hours of planing ;)
Jun 27 '08 #6
christian.bau wrote:
Another recommendation: Your combination of using strncpy to fill an
array of char and strcmp to compare the contents is dangerous. Either
your strings are limited to five characters and fit, then use strcpy.
Or they can be longer than five characters, then the call to strcmp is
dangerous and can give the wrong result.
Yes i know this but BusID will always be 5 characters only. It'll always be
in the form "X1234" so I should be safe with strncpy() and strcmp(). But
thanks for pointing this out!

Ron
--
weeks of software enineering safe hours of planing ;)
Jun 27 '08 #7
Ron Eggler <un*****@example.comwrites:
christian.bau wrote:
>Another recommendation: Your combination of using strncpy to fill an
array of char and strcmp to compare the contents is dangerous. Either
your strings are limited to five characters and fit, then use strcpy.
Or they can be longer than five characters, then the call to strcmp is
dangerous and can give the wrong result.

Yes i know this but BusID will always be 5 characters only. It'll always be
in the form "X1234" so I should be safe with strncpy() and strcmp(). But
thanks for pointing this out!
strncpy() is rarely what you want. It isn't *really* a string
function. A string is an arbitrary sequence of characters, terminated
by and including a trailing '\0'. The data structure that strncpy()
deals with (at least in its target) is a fixed-length sequence of
characters ending in zero or more '\0's.

You probably might as well use strcpy() (or perhaps memcpy()).

--
Keith Thompson (The_Other_Keith) ks***@mib.org <http://www.ghoti.net/~kst>
Nokia
"We must do something. This is something. Therefore, we must do this."
-- Antony Jay and Jonathan Lynn, "Yes Minister"
Jun 27 '08 #8
On Jun 5, 7:54*pm, Ron Eggler <unkn...@example.comwrote:
Well the reason for me using a double pointer is, because i want to use the
array elements from my main loop and this specific struct variable (in the
array) should keep its value from time of allocation until it is destroyed
again in ClearElement().
Does this make more sense now?
No. Your data is based on two values: One is a variable "StructCount",
one is a malloc'd pointer to that number of MsgStruct structures. This
two values are logically inseparable, but you are separating them.
Why? You not only keep them separate, you pass them to your two
functions in different ways: One as a global variable (apparently),
and one by passing it's address. Either use two globals, or pass the
address of StructCount, or put StructCount and the pointer together
into one struct.
Jun 27 '08 #9

This thread has been closed and replies have been disabled. Please start a new discussion.

Similar topics

6
by: muser | last post by:
The following error appears: 'strcmp' : cannot convert parameter 1 from 'char' to 'const char *'. I've already tried using single quotations. the header file only contains the struct contents....
3
by: jl_post | last post by:
Hi, I recently wrote two benchmark programs that compared if two strings were equal: one was a C program that used C char arrays with strcmp(), and the other was a C++ program that used...
53
by: Allan Bruce | last post by:
Hi there, I am reading a file into a char array, and I want to find if a string exists in a given line. I cant use strcmp since the line ends with '\n' and not '\0'. Is there a similar function...
11
by: Eirik | last post by:
Shouldn't this code work? If not, why shouldn't it? #include <stdio.h> int main(void) { char yesno; char *yes = "yes";
9
by: Steven | last post by:
Hello, I have a question about strcmp(). I have four words, who need to be compared if it were two strings. I tried adding the comparison values like '(strcmp(w1, w2) + strcmp(w3, w4))', where...
36
by: Chuck Faranda | last post by:
I'm trying to debug my first C program (firmware for PIC MCU). The problem is getting serial data back from my device. My get commands have to be sent twice for the PIC to respond properly with...
0
by: noobcprogrammer | last post by:
#include "IndexADT.h" int IndexInit(IndexADT* word) { word->head = NULL; word->wordCount = 0; return 1; } int IndexCreate(IndexADT* wordList,char* argv)
47
by: fishpond | last post by:
One way I've seen strcmp(char *s1, char *s2) implemented is: return immediately if s1==s2 (equality of pointers); otherwise do the usual thing of searching through the memory at s1 and s2. Of...
2
by: thungmail | last post by:
There is partial code in C typedef struct message { int messageId; char *messageText; struct message *next; }message; ..... ..... ..... /* Get a node before a node */
0
isladogs
by: isladogs | last post by:
The next Access Europe meeting will be on Wednesday 6 Mar 2024 starting at 18:00 UK time (6PM UTC) and finishing at about 19:15 (7.15PM). In this month's session, we are pleased to welcome back...
1
isladogs
by: isladogs | last post by:
The next Access Europe meeting will be on Wednesday 6 Mar 2024 starting at 18:00 UK time (6PM UTC) and finishing at about 19:15 (7.15PM). In this month's session, we are pleased to welcome back...
0
by: jfyes | last post by:
As a hardware engineer, after seeing that CEIWEI recently released a new tool for Modbus RTU Over TCP/UDP filtering and monitoring, I actively went to its official website to take a look. It turned...
0
by: ArrayDB | last post by:
The error message I've encountered is; ERROR:root:Error generating model response: exception: access violation writing 0x0000000000005140, which seems to be indicative of an access violation...
1
by: PapaRatzi | last post by:
Hello, I am teaching myself MS Access forms design and Visual Basic. I've created a table to capture a list of Top 30 singles and forms to capture new entries. The final step is a form (unbound)...
1
by: CloudSolutions | last post by:
Introduction: For many beginners and individual users, requiring a credit card and email registration may pose a barrier when starting to use cloud servers. However, some cloud server providers now...
1
by: Defcon1945 | last post by:
I'm trying to learn Python using Pycharm but import shutil doesn't work
1
by: Shællîpôpï 09 | last post by:
If u are using a keypad phone, how do u turn on JavaScript, to access features like WhatsApp, Facebook, Instagram....
0
by: Faith0G | last post by:
I am starting a new it consulting business and it's been a while since I setup a new website. Is wordpress still the best web based software for hosting a 5 page website? The webpages will be...

By using Bytes.com and it's services, you agree to our Privacy Policy and Terms of Use.

To disable or enable advertisements and analytics tracking please visit the manage ads & tracking page.