Thread: Possible memory leak?

I created a function - primarily to remove quotes - that removes the first and last character from a string if the character sent to the function matches those first and last of the sent string. However, Since I end up incrementing a pointer I was wondering if I had sent the function an allocated array (say a "char String[100]") and the first and last character are removed would trying to access element 99 cause a memory leak?

Code:

  int Remove_Outer
  (
          char *String,
    const char Outer
  ){
    int Last_Index = strlen(String) - 1;
    if(Last_Index < 0)
      return FAILURE;
    if(String[0] != Outer || String[Last_Index] != Outer)
      return FAILURE;
    String[Last_Index] = '\0';
    String++; /* May cause memory leak here */
    return SUCCESS;
  }

Can I do a shifting operation on the bytes to move the characters over?

char string[100] is not allocated on heap. there's nothing to leak.
For heap allocated, as long as you have the pointer to allocated heap. It's not leak.
Don't forget that incrementing string in your function cannot even modify what the pointer points in caller.

eg.

Code:

void foo( char *s) 
{
   s += 5;    // assume ok to inc 5
}

char s[] = "hello world";
foo(s);
printf("%s\n",s);       // s still points to "hello world" not " world"

Originally Posted by 127.0.0.1

Can I do a shifting operation on the bytes to move the characters over?

See my example in message 2.

Sorry, I am still relatively new to C Tater. If you could add some comments to your example to help me understand it would be very much appreciated.

Originally Posted by Salem

Sure, you can do s[n] = s[n+1] in a loop.

And if the string has 3 quotation marks at the beginning or 2 at the end?

Originally Posted by 127.0.0.1

Sorry, I am still relatively new to C Tater. If you could add some comments to your example to help me understand it would be very much appreciated.

Code:

void unquote(char *str)
  { char *h = str; // safe head and end pointers
     char *e = str; 
     while (*h)               // exit on trailing null
       { if (*e == '\"')      // if we find a quote
            e++;              // increment the end pointer
          else 
            *(h++) = *(e++); } }  // otherwise copy the character.

Basically it's setting two pointers to the first part of the string.
Each time a quotation mark is found we increment the end pointer skipping that character.
If the character is not a quotation mark we copy it to the head pointer and increment both pointers.
The loop terminates when we copy the trailing null at the end of the string.

And if the string has 3 quotation marks at the beginning or 2 at the end?

This isn't actually a problem for my specific need, but it is always helpful to study code you don't understand. So I think I am going to use the simple method and shift the characters over.

Here is my final function:

Code:

  int Remove_Outer
  (
          char *String,
    const char Outer
  ){
    int Last_Index = strlen(String) - 1;
    if(Last_Index < 0)
      return FAILURE;
    if(String[0] != Outer || String[Last_Index] != Outer)
      return FAILURE;
    String[Last_Index] = '\0';
    for(int i = 1;i <= Last_Index;i++)
      String[i - 1] = String[i];
    return SUCCESS;
  }

Thanks again for all of the help.

Edit: Is it possible to use bit shifting to accomplish this as well?

Originally Posted by 127.0.0.1

This isn't actually a problem for my specific need, but it is always helpful to study code you don't understand. So I think I am going to use the simple method and shift the characters over.

Here is my final function:

Code:

  int Remove_Outer
  (
          char *String,
    const char Outer
  ){
    int Last_Index = strlen(String) - 1;
    if(Last_Index < 0)
      return FAILURE;
    if(String[0] != Outer || String[Last_Index] != Outer)
      return FAILURE;
    String[Last_Index] = '\0';
    for(int i = 1;i <= Last_Index;i++)
      String[i - 1] = String[i];
    return SUCCESS;
  }

Thanks again for all of the help.

Edit: Is it possible to use bit shifting to accomplish this as well?

No you won't do this with bit shifting... each charater is in a different byte. What you're doing is byte shifting.

That said, I really think you should spend some time with your function in a debugger... Frankly, reading it is making me dizzy and you are using Outer in an uninitialized state.

Originally Posted by CommonTater

Frankly, reading it is making me dizzy

Is this a comment on my style?

Originally Posted by CommonTater

... you are using Outer in an uninitialized state.

What do you mean by using Outer in an "uninitialized state"? Are the negative consequences for this, and what is recommended instead?

Originally Posted by 127.0.0.1

Is this a comment on my style?

Well, sort of, since it's rather hard to follow what you're doing...

What do you mean by using Outer in an "uninitialized state"? Are the negative consequences for this, and what is recommended instead?

Code:

const char Outer;

// then first use
if(String[0] != Outer || String[Last_Index] != Outer)

What is the value of Outer at that moment?

Originally Posted by 127.0.0.1

It is passed to the function in the call - in the case discussed it would be the \" character. The call would look like this:

Code:

Remove_Outer(String, '\"');

Edit: It first check to see if it the character passed does indeed surround the string, if not it returns FAILURE because there is no removal operation to be done.

Ahh... ok, I see that now... it's your unusual text setup... I initially missed the bracketing in your function definition...
Most often these things are written on a single line or consecutive lines if they are too long... it is exceedingly rare to see a function definition spaced out like that... A far more readable version would be...

Code:

 int Remove_Outer  ( char *String, const char Outer )
   {
    int Last_Index = strlen(String) - 1;

    if(Last_Index < 0)
      return FAILURE;

    if(String[0] != Outer || String[Last_Index] != Outer)
      return FAILURE;

    String[Last_Index] = '\0';

    for(int i = 1;i <= Last_Index;i++)
      String[i - 1] = String[i];

    return SUCCESS;
  }

The compiler may not care a rat's behind about text setup... but we humans sure do.