-2

I'm having trouble getting strcmp to return 0 for equal strings.

This is how my code looks (the relevant part anyway):

struct person{
    char name[30];
    time_t date;
    char destination[30];
    char phone[30];
};

in the main i have:

struct person act_person;
int f = open("persons.txt", O_RDONLY | O_CREAT, S_IRUSR);
if(f == -1){
    //open error
}
char city_name[30]; 
scanf(" %[^\n]s", city_name);
city_name[strcspn(city_name, "\n")] = 0;
//lseek(f, 0, SEEK_SET); //took it out as user alk suggested
while(read(f, &act_person, sizeof(act_person))){ 
     act_person.destination[strcspn(act_person.destination, "\n")] = 0;
     if(strcmp(act_person.destination,city_name)==0){
            printf("%s\n",act_person.name);
     }
}
close(f);

The file I'm reading from was written to in similar byte sized chunks of person structs so it should work... yet no matter what i do strcmp will not return equal.

edit: I'm going to try to show a hexdump of the file to help find the problem as the user suggested, as soon as I figure out how to do just that

here is the dump:

Hexdump of persons.txt

user1966576
  • 83
  • 1
  • 10
  • 2
    You should so some debugging - check the byte values contained in each of your strings. – Oliver Charlesworth Mar 27 '16 at 18:00
  • 4
    General comment: I understand you're frustrated, but the complaints at the end don't add much to the question. They just make me think "This person is angry. If I try to help, are they just going to get angry at me?" – Nate Eldredge Mar 27 '16 at 18:01
  • You may try `strcmpi` to get rid of upper/lower case issues... – Lukas Thomsen Mar 27 '16 at 18:01
  • 2
    So you may want to calm down and then reread your question to make sure it contains everything that someone would need to be able to help. Note that phrases like "it doesn't work" are vague and not helpful - what happens exactly? It would also help to make a [minimal complete verifiable example](http://stackoverflow.com/help/mcve) - right now nobody can test your code, because you have left out parts, and we also don't know what the input file contains. – Nate Eldredge Mar 27 '16 at 18:03
  • 1
    Show us a `hexdump -C` for the files 1st, let's say, 128 bytes. – alk Mar 27 '16 at 18:07
  • 3
    Learn how to use GDB. It's an invaluable tool when working with C. Also, I highly recommend NOT having 3 function calls on the same line when you're running into these issues. Try something like `char *act_person_tok = ...` as well as the other two calls also on separate lines. Maybe print them in between, or use GDB's `p` function. Best of luck with debugging. Also, C is a "frustrating" language because it doesn't hold your hand. If you require your hand being held, go use something else. Simple as that. Being angry on this site won't get you anywhere. – Ricky Mutschlechner Mar 27 '16 at 18:09
  • I just see the file's name carries the extension `.txt`, which makes me assume it's just a plain text file. If this is the case you won't get far by reading directly into a binary `struct`. Use `fgets()` instead to read the file line by line and parse the lines as necessary. – alk Mar 27 '16 at 18:13
  • You're using `strtok()` in manner that it's know to have problems, consider using `strtok_r()` instead, if available, or replacing `strtok()` with something else. – cdlane Mar 27 '16 at 18:13
  • K I added the hexdump. Mark and James are 2 ppl in the file (the only 2) all of which have a destination and phone numbers. But if i enter in "Mark" or "James" the strcmp does not return equal. – user1966576 Mar 27 '16 at 18:17
  • BTW, to chop of new-lines from the "string" `buffer` the most sane approach is to do `buffer[strcspn(buffer, "\n")] = 0;`. – alk Mar 27 '16 at 18:18
  • From the hex-dump a record is 0x68 bytes long. What does `sizeof (struct person)` return on your system? – alk Mar 27 '16 at 18:23
  • Drop the `strtok()`ing on `act_person`'s members there are no new-line read. The file is binary. – alk Mar 27 '16 at 18:25
  • I tried that buffer chop, still won't work. The sizeof returns 104. – user1966576 Mar 27 '16 at 18:27
  • Also add error checking to the calls to `open()` and `read()`. Both return `-1` on error. Drop the `lseek()`. – alk Mar 27 '16 at 18:28
  • added error checking to read, dropped lseek. Still doesn't work. – user1966576 Mar 27 '16 at 18:33
  • Add your current code as update to your question. Also "*does not work*" is the worth error description possible .. really. – alk Mar 27 '16 at 18:34
  • Does `open()` succeed? – alk Mar 27 '16 at 18:36
  • Yes open succeeds. – user1966576 Mar 27 '16 at 18:36
  • You are aware that your code is comparing `destination` to the name entered? – alk Mar 27 '16 at 18:38
  • yes but name is the name of a destination in this case... sorry if that was not clear. I'll change that too. I also updated the code. – user1966576 Mar 27 '16 at 18:40
  • So you enter `Mark`? – alk Mar 27 '16 at 18:41
  • 1
    0x68 bytes: `name[30]`, 2 bytes of padding, 64-bit `time_t` value (8 bytes), `destination[30]`, 2 bytes of padding, `phone[30]`, 2 bytes of padding. So that's 30+2 + 8 + 30+2 + 30+2 = (30+2)*3 + 8 = 96+8 = 104 (hexadecimal: 0x68). Hopefully that matches up (test whether all names print properly to verify that things are being read properly). If it doesn't, then you have a problem somewhere, such as a 32-bit `time_t` or more padding bytes in your structure than expected. –  Mar 27 '16 at 18:41
  • @ChronoKitsune: The guy confirmed the 104 here http://stackoverflow.com/questions/36250685/strcmp-does-not-return-equal-on-equal-strings#comment60131631_36250685 – alk Mar 27 '16 at 18:42
  • @alk Ah, I didn't see that it had been confirmed. My mistake. –  Mar 27 '16 at 18:44
  • There is no usefull data for destination in side your test file. Look at the hexdump: `fc 21 f8 ...` this is garbage, at least no printable characters. Any no city name ... I see only "Mark" and "James". – alk Mar 27 '16 at 18:45
  • @alk no you don't enter mark. the "name" variable really should be city_name or something, so you enter "London" for example, and the act_person.destination in the file should also have "London" but it doesn't work. – user1966576 Mar 27 '16 at 18:45
  • What i completely don't understand is, I do this exact same process with another file with another function in the program, and strcmp works there.... the only difference is, there I compare two struct members (both of which are char arrays), and here I am comparing a struct member and a character array... – user1966576 Mar 27 '16 at 18:54
  • 1
    The issue is not with strcmp but with the data you pass in to it. As I said: At least in the dump you show there is no useful "string" data for destination around, at least none which you can compare to anything you are able to enter via the keyboard via scanf. The latter assumes ASCII encoding, which is the most common and also seem to be used for "name", as "Mark" and "James" are clearly visible. – alk Mar 27 '16 at 18:59

2 Answers2

1

So this will read the first record from your file into act_person. The data from the file will give you:

.name = "Mark"
.date = 0x56f821fc
.destination = "\xb5\xd9\xc1\x95\xac\x7f"
.phone = "361444555"

You then compare the name your read the input with the desitination field and not suprisingly, they don't match. The destination of the next record similarly seems to be garbage.

Chris Dodd
  • 119,907
  • 13
  • 134
  • 226
  • I suspect you are slightly wrong with the destination. The bytes you show are from offset 8 of destination. – alk Mar 27 '16 at 19:04
  • Assuming destination starts with record offset 32: The 1st record's destination start with `0xfc 0x21`. – alk Mar 27 '16 at 19:05
  • Err ok i have just now fixed part of the problem. Now strcmp seems to work, but only for the last "struct" syzed bytes on the file. – user1966576 Mar 27 '16 at 19:09
  • I was under the impression that while(read(etc etc)) would read the entire file, one struct syzed data at a time, but for some reason it only seemingly reads the last entry? – user1966576 Mar 27 '16 at 19:10
  • @alk: date will be at offset 32 (0x20), and destination at offset 40 (0x28), phone at offset 70 (0x46). – Chris Dodd Mar 28 '16 at 02:40
  • You are absolutely correct. Too late, too small font, whatever ... :} – alk Mar 28 '16 at 07:43
-1

  1. Calling read with the address of a struct is dangerous. There is absolutely no guarantee about the layout of a struct. Different byte ordering, different padding, different size, time_t is implementation defined. You should really change this.

  2. strtok modifies the string that it examines. Do you want that? Especially with the string "name". It is modified by the strtok call.

  3. Instead of doing a complicated if statement, store values into intermediate variables, so you can use a debugger to check what is happening. What are the values before and after the strtok calls?

  4. Searching for an hour for a solution shouldn't be frustrating.

(Just noticed the question has changed since I typed the answer).

  1. Random changes rarely help. Examine what happens via debugging. Understand what's going wrong. Figure out how to figure the problem. Then fix the problem.
gnasher729
  • 51,477
  • 5
  • 75
  • 98
  • Ya i took out strtok and used what the other guy suggested instead. I'll try seeing what the values are in a moment. – user1966576 Mar 27 '16 at 18:43