3
\$\begingroup\$

I'm trying to write a function that will work out the length of two string char arrays, using pointer arithmetic. It seems to be working but I'm still getting used to pointers and addresses etc and would be grateful if you could let me know what you think. If possible could tell me if my understanding is correct in thinking that, when finding the length of a string char array, you're actually finding how many bytes it is? Thank you.

Here is the code:

#include <stdio.h> #include <stdlib.h> int cmpstr(const char *,const char *); int main() { const char hello[10] = "hello"; const char world[10] = "world"; printf("%d", cmpstr(hello, world)); return 0; } int cmpstr(const char *s1, const char *s2) { int start1 = s1; // start address for s1 string while(*s1 != '\0') { s1++; // increase address until the end } s1 = s1-start1; // subtract start address from end address int start2 = s2; // start address for s1 string while(*s2 != '\0') { s2++; // increase address until the end } s2 = s2-start2; // subtract start address from end address if(s1==s2) // if s1 string is equal to s2 string { return 0; } else if(s1>s2) // if s1 string is greater than s2 string { return 1; } else if(s1<s2) // if s1 string is less than s2 string { return -1; } } 
\$\endgroup\$

    2 Answers 2

    5
    \$\begingroup\$

    type of difference between pointers

    pointer_one - pointer_two 

    The result of this expression is a ptrdiff_t (§6.5.6/9). Assigning it back to a pointer converts an integer type to a pointer, which IIRC should give you a compiler warning as it can lead to all kind of trouble.

    relational operators with pointers

    Comparing pointers with relative operators is undefined behavior if the pointers don't point into the same array (§6.5.8/5).

    else if(s1>s2) // undefined behavior { return 1; } else if(s1<s2) // undefined behavior { return -1; } 

    naming

    cmpstr isn't really a suitable name for a function that compares the lengths of two strings.

    comments

    // if s1 string is equal to s2 string

    You don't compare strings for equality. You (try to) compare their lengths.

    Also, your comments should explain why you wrote the code the way you did. Don't tell what the code is doing. With well written code that should be self apparent.

    do one thing per function

    cmpstr calculates the length of two strings (code duplication!) and then compares these. That's too much. Delegate that length calculation to another function: strlen. Or - for learning purpose only - write your own replacement.

    ptrdiff_t vs size_t

    Wondered why strlen returns a size_t instead of a ptrdiff_t? That's because a string might be longer than the maximum difference that can be represented by ptrdiff_t. Thus end - start may actually be undefined behavior when done with really long strings. Better keep a size_t counter that you increase while searching for the '\0'.

    program output should end with a '\n'

    This is only minor, but otherwise it may corrupt the controlling terminal a bit. Or like in ideone drop the output completely.

    printf("%d\n", cmpstr(hello, world)); // ^^ 

    test your code

    You really should! Every (in an ideal world) piece of code that you write should be tested. You might want to look into TDD (test driven development) and build up good habits from the start.

    making it faster

    Do you really need to obtain the length of both strings? Take these two strings as example:

    "" "the quick brown fox did something but I don't remember what" 

    When you start at the first char of each of those, at how many chars do you need to look to determine which string is longer?

    \$\endgroup\$
    0
      1
      \$\begingroup\$

      The first thing compare code attempts is to assign a pointer to an int.

      An int may not save all the information inform a pointer. Example: a 64-bit pointer saved to a 32-bit int certainly loses information.

      int cmpstr(const char *s1, const char *s2) { int start1 = s1; // start address for s1 string 

      If code needs to save the value of s1, simple use a variable of the same type.

       // int start1 = s1; const char * start1 = s1; 

      Various weak coding practices are rapidly detected when the compiler warnings are fully enabled.

       int start1 = s1; // start address for s1 string // warning: initialization makes integer from pointer without a cast [-Wint-conversion] } // warning: control reaches end of non-void function [-Wreturn-type] 

      As code ends without an obvious return, better to insure a final return.

       // if (s1 == s2) { // return 0; // } else if (s1 > s2) { // return 1; // } else if (s1 < s2) { // return -1; // } if (s1 == s2) { return 0; } if (s1 > s2) { return 1; } return -1; 

      when finding the length of a string char array, you're actually finding how many bytes it is?

      Consider comparing the lengths of the 2 strings as code marches down them. No need to compute nor save the length of the strings.

      int cmpstr_length(const char *s1, const char *s2) { while (*s1 && *s2) { s1++; s2++; } if (*s2) { return -1; } if (*s1) { return 1; } return 0; } 
      \$\endgroup\$

        Start asking to get answers

        Find the answer to your question by asking.

        Ask question

        Explore related questions

        See similar questions with these tags.