1
\$\begingroup\$

I wanted to create a function which accepts a string & a delimiter. Finds the small blocks of string separated by the delimiters and replace the block based on some logic.

I have created a simple example below to represent the logic I am trying to write. I also tried to make sure that I handle all cases where the user can by-mistakely pass a lone delimiter.

Eg. Usage: 

findDelimitedBlocks(inString,"@@#")
inString: @@#animal@@# is a great hunter, he likes @@#food@@#.
outString: LION is a great hunter, he likes deer.....

API:

void main()
{
     char  queryInStringg[10000] = "@@#animal@@# is a great hunter, he likes @@#food@@#....";

     printf("\nString Before [%s]",queryInStringg);fflush(stdout);

     findDelimitedBlocks(queryInStringg,"@@#");

     printf("\nString After [%s]",queryInStringg);fflush(stdout);

    return 0;
}

int findDelimitedBlocks(char *inString, char *delimiter)
{
    char  string_Variable[5000]              ;
    char  outString[strlen(inString)+5000]   ;
    long  string_Variable_Index  = 0         ;
    long  inStringIndex          = 0         ;
    long  outStringIndex         = 0         ;
    int   logical_Variable = 0               ;
    long  i_loop_counter = 0                 ;
    long  j_loop_counter = 0                 ;
    long  delimiter_Size = 0                 ;

    memset(string_Variable, 0x00, sizeof(string_Variable));
    memset(outString, 0x00, sizeof(outString));

    delimiter_Size = strlen(delimiter);

    while(1)
    {
        if(inString[inStringIndex] == 0x00)
        {
            outString[outStringIndex] = 0x00;
            break;
        }

        if(inString[inStringIndex] == delimiter[0])
        {
            for(i_loop_counter = 1 ; ; i_loop_counter++)
            {
                if(inString[inStringIndex+i_loop_counter] == 0x00)
                {
                    logical_Variable = 0;
                    break;
                }

                if(delimiter[i_loop_counter] == 0x00)
                {
                    logical_Variable = 1;
                    break;
                }

                if(inString[inStringIndex+i_loop_counter] != delimiter[i_loop_counter])
                {
                    logical_Variable = 0;
                    break;
                }
            }

            if(logical_Variable == 1)
            {
                memset(string_Variable, 0x00, sizeof(string_Variable));
                string_Variable_Index = 0;
                logical_Variable = 0;

                for(j_loop_counter = 0 ; ; j_loop_counter++)
                {
                    if(inString[inStringIndex+delimiter_Size+j_loop_counter] == 0x00)
                    {
                        outString[outStringIndex++] = inString[inStringIndex];
                        break;
                    }

                    if(inString[inStringIndex+delimiter_Size+j_loop_counter] == delimiter[0])
                    {

                        for(i_loop_counter = 1 ; ; i_loop_counter++)
                        {

                            if(delimiter[i_loop_counter] == 0x00)
                            {
                                logical_Variable = 1;
                                break;
                            }

                            if(inString[inStringIndex+delimiter_Size+j_loop_counter+i_loop_counter] != delimiter[i_loop_counter]
                            || inString[inStringIndex+delimiter_Size+j_loop_counter+i_loop_counter] == 0x00)
                            {
                                logical_Variable = 0;
                                break;
                            }
                        }

                        if(logical_Variable == 1)
                        {
                            string_Variable[string_Variable_Index] = 0x00;

                            /*After Finding I will incorporate the rest of the replace logic*/
                            if(strcmp(string_Variable,"animal") == 0)
                            {
                                memset(string_Variable, 0x00, strlen(string_Variable));
                                sprintf(string_Variable,"LION");
                            }
                            else if(strcmp(string_Variable,"food") == 0)
                            {
                                memset(string_Variable, 0x00, strlen(string_Variable));
                                sprintf(string_Variable,"deer");
                            }
                            else
                            {
                                memset(string_Variable, 0x00, strlen(string_Variable));
                                sprintf(string_Variable,"IREPLACE");

                            }

                            strcat(outString,string_Variable);
                            outStringIndex += strlen(string_Variable);
                            inStringIndex += string_Variable_Index+delimiter_Size+delimiter_Size - 1;
                            break;
                        }
                        else
                        {
                            string_Variable[string_Variable_Index++] =  inString[inStringIndex+delimiter_Size+j_loop_counter];
                        }

                    }
                    else
                    {
                        string_Variable[string_Variable_Index++] =  inString[inStringIndex+delimiter_Size+j_loop_counter];
                    }
                }

            }
            else
            {
                outString[outStringIndex++] = inString[inStringIndex];
            }

        }
        else
        {
            outString[outStringIndex++] = inString[inStringIndex] ;
        }

        inStringIndex ++ ;
    }

    memset(inString, 0x00, strlen(inString));
    strcpy(inString,outString);

    return(0);
}

I want to know

  • If there are any more exceptions that I might have missed or there is any bug in the code?
  • Is there a better/more efficient way to write the function?
\$\endgroup\$
0

1 Answer 1

Reset to default
1
\$\begingroup\$

The interface is dangerous

Functions that write to user-provided memory absolutely need to know how much memory is valid, and not write beyond the bounds. I could write something like this:

 char s[] = "some string with tags";
 findDelimitedBlocks(s, "@");

And if the replacements are larger than the tags, then we'll write beyond the bounds of s. We really need another argument, size_t max, that specifies how much of inString is available for writing, in the same way that snprintf() and strncpy do.

I think perhaps it might be better for the caller to provide outString separately from inString (which could then be const char* - e.g. a string literal), giving the option whether or not to copy back and overwrite the original.

Also, we want to make the function general, so that the list of replacements can be passed as a parameter.

Use standard library strstr()

We seem to be painfully reimplementing string search when we're looking for delimiter. I think we could simplify the code greatly by using strstr(); we then know how much to directly strcpy to output, or to search for amongst the replacements.

Remove pointless memset()

Here, memory is immediately overwritten, and the first line achieves nothing:

        memset(string_Variable, 0x00, strlen(string_Variable));
        sprintf(string_Variable,"LION");

Nitpick

int main(void), not void main().

Working replacement

Here's how I'd write this, taking into account the comments I made:

#include <stdbool.h>
#include <string.h>

/*
   Attempt to copy COUNT chars from SRC to DEST without exceeding
   CAPACITY (including terminating NUL char).

   Update capacity and dest to reflect the remaining space.

   Return the amount that would be written if capacity were unlimited.
 */
static size_t write_and_update(char **dest, size_t *capacity,
                               const char *src, size_t count)
{
    if (!dest || !count) {
        return count;
    }

    if (count < *capacity) {
        strncpy(*dest, src, count);
        *dest += count;
        *capacity -= count;
    } else if (*capacity) {
        strncpy(*dest, src, *capacity);
        (*dest)[*capacity-1] = '\0';
        *dest = NULL;
        *capacity = 0;
    }

    return count;
}


struct replacement {
    const char *tag;
    const char *text;
};

/* return length of substituted string (perhaps greater than capacity). */
/* dest may be NULL only if capacity is zero */
size_t replace_in_string(char *dest, size_t capacity,
                         const char *src, const char *delimiter,
                         size_t rep_count, const struct replacement *reps)
{
    const size_t delim_len = strlen(delimiter);

    size_t output_size = 0;

    while (true) {
        /* find two delimiters */
        const char *const delim_start = strstr(src, delimiter);
        if (!delim_start) break;
        const char *const tag_start = delim_start + delim_len;

        const char *const tag_end = strstr(tag_start, delimiter);
        if (!tag_end) break;
        const char *const delim_end = tag_end + delim_len;

        const size_t tag_len = (size_t)(tag_end - tag_start);

        /* write the prefix part */
        const size_t text_len = (size_t)(delim_start - src);
        output_size += write_and_update(&dest, &capacity, src, text_len);

        /* look up the replacement */
        /* linear search, but we could use binary search if rep_count gets large */
        const struct replacement *rep = NULL;
        for (size_t i = 0;  i < rep_count;  ++i) {
            if (strlen(reps[i].tag) == tag_len && memcmp(reps[i].tag, tag_start, tag_len) == 0) {
                rep = &reps[i];
                break;
            }
        }
        if (!rep) {
            /* not found */
            const size_t delim_len = (size_t)(delim_end - delim_start);
            output_size += write_and_update(&dest, &capacity, delim_start, delim_len);
        } else {
            output_size += write_and_update(&dest, &capacity, rep->text, strlen(rep->text));
        }

        /* update */
        src = delim_end;
    }

    {
        /* write the remaining text */
        output_size += write_and_update(&dest, &capacity, src, strlen(src));
    }

    return output_size;
}

    

#include <stdio.h>
#include <stdlib.h>

int main(void)
{
    const char *template = "@@#animal@@# is a great hunter, he likes @@#food@@#....";
    printf("String Before [%s]\n", template);

    static const struct replacement replacements[] = {
        { "animal", "LION" },
        { "food", "deer" },
    };

    const size_t rep_len = sizeof replacements / sizeof *replacements;

    const size_t out_size = replace_in_string(NULL, 0, template, "@@#",
                                              rep_len, replacements);

    char *output = malloc(out_size+1);
    if (!output) {
        fprintf(stderr, "Allocation of %zu failed!", out_size);
        return 1;
    }

    replace_in_string(output, out_size, template, "@@#", rep_len, replacements);
    printf("String After [%s]\n", output);

    free(output);
}
\$\endgroup\$
1
  • \$\begingroup\$ You are right. Thank you so much for the replacement. Your approach was enlightening. \$\endgroup\$ Commented Nov 9, 2018 at 13:49

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.