Sagar
Sagar

Reputation: 9503

Can't figure out why this trimming function won't work correctly

===============================================================================

void trim(const char * orig, char * dest)
{
    size_t front = 0;
    size_t end = sizeof(orig) - 1;
    size_t counter = 0;
    char * tmp = null;

    if (sizeof(orig) > 0)
    {
        memset(dest, '\0', sizeof(dest));

        /* Find the first non-space character */
        while (isspace(orig[front]))
        {
                front++;
        }
        /* Find the last non-space character */
        while (isspace(orig[end]))
        {
                end--;
        }

        tmp = strndup(orig + front, end - front + 1);
        strncpy(dest, tmp, sizeof(dest) - 1);
        free(tmp); //strndup automatically malloc space
    }
}

===============================================================================

I have a string:

'     ABCDEF/G01        '

The above function is supposed to remove the spaces and return to me:

'ABCDEF/G01'.

Instead, what I get back is:

'ABCDEF/'

Any ideas?

Note: the quotes are just to show you that spaces exist in the original string.

Upvotes: 3

Views: 102

Answers (6)

ASten
ASten

Reputation: 814

You must replace sizeof() to strlen() everywhere in your function. Here is working edit:

void trim(const char * orig, char * dest)
{
    size_t front = 0;
    size_t end = strlen(orig)-1;
    size_t counter = 0;
    char * tmp = NULL;

    if (strlen(orig) > 0)
    {
        memset(dest, '\0', strlen(dest));

        /* Find the first non-space character */
        while (isspace(orig[front]))
        {
            front++;
        }
        /* Find the last non-space character */
        while (isspace(orig[end]))
        {
            end--;
        }

        tmp = strndup(orig + front, end - front + 1);
        strncpy(dest, tmp, strlen(dest));
        free(tmp); //strndup automatically malloc space
    }
}

(I've tested it)

Upvotes: 0

INS
INS

Reputation: 10820

Try this code (it doesn't use temporary memory):

void trim(const char * orig, char * dest)
{
    size_t front = 0;
    size_t end = strlen(orig)-1;
    size_t counter = 0;

    *dest = '\0';

    if (strlen(orig) > 0)
    {    
        /* Find the first non-space character */
        while (front < end && isspace(orig[front]) )
        {
                front++;
        }
        /* Find the last non-space character */
        while (front < end && isspace(orig[end]))
        {
                end--;
        }

        counter = front;
        while ( counter <= end )
        {
                dest[counter-front] = orig[counter];
                counter++;
        }
    }
}

NOTE: Not tested!

Upvotes: 0

Some programmer dude
Some programmer dude

Reputation: 409472

The sizeof(dest) doesn't do what you think it does! It returns the size of the pointer, not the length of the string. You need to supply a maximum length of the destination to your function.

For the string orig you want to use the strlen function.

Upvotes: 2

K-ballo
K-ballo

Reputation: 81409

void trim(const char * orig, char * dest)
{
    size_t front = 0;
    size_t end = sizeof(orig) - 1;

In that code, sizeof(orig) is the size of a pointer. All pointers are the same size, probably 8 in your implementation. What you want to use is strlen(orig) instead.

Upvotes: 1

Giovanni Funchal
Giovanni Funchal

Reputation: 9200

size_t end = sizeof(orig) - 1;
strncpy(dest, tmp, sizeof(dest) - 1);

You probably want strlen instead of sizeof here.

Upvotes: 1

cnicutar
cnicutar

Reputation: 182754

The strncpy is wrong. sizeof(dest) is not what you want (it's the size of a pointer on your machine). You probably want: end - front. Instead, try:

memcpy(dest, front + start, end - front);
dest[end] = 0;

Upvotes: 4

Related Questions