Thread: insertstr

I've got a strong feeling something is wrong with this code but as I presently cannot see it I thought I would post it here and see if anyone spots anything overnight, if no-one replies by 2mw evening then I'll just risk it and try it in the runtime.

Code:

IPIPE_EXP char* insertstr( char *dst, char const *src, size_t size, size_t pos )
{
  size_t sc = strlen( src ) - 1, dc = strlen( dst ) - 1, i = dc + sc;

  if ( i >= size )
  {
    sc -= ( i - size );
    i = size - 1;
  }

  size_t j = dc;
  src = ( src + sc );

  for ( ; j > pos; --i, --j, --src )
  {
    dst[i] = dst[j];
    dst[j] = *src;
  }

  i = ( dc + sc ) + 1;

  if ( i == size )
  {
    dst[ --i ] = 0;
  }
  else
  {
    dst[i] = 0;
  }

  return dst;
}

And yes I'm aware I do not check the parameters at the start, only 1 instance of it's usage atm and the parameters are definitely valid there, I'll sort out parameter checks later when I'm done with 95% of the app I'm making (the point where the user chooses libraries to fill the last 5% - app hooks and format support)

It would help if you provided a few more details, like what input you give that causes it to not work, what output you get, and what output you expect to get. It would also help if you explained what IPIPE_EXP is/does.

Have you tried running it through a debugger, and examining all the variables at crucial points in your algorithm, to see what it is doing right/wrong? A debugger is one of the most useful tools for any programmer. Learning to use one, even just the basics, is invaluable.

Also, you should try tracing the code by hand. Learning to trace code by hand is a crucial skill. What I usually do is make a table that has all the variables and tracks their value after each statement. Initially, this is a tedious exercise, but eventually it becomes second nature, and you can easily track large, complex functions in your head. I'm using ? to mean unknown or no value yet, not as a literal '?' character. I am putting a . in front of a string to show how many chars before a the place a pointer points into a string.

Code:

// sample call
char dst[20] = "foobar";
char src[20] = "xxx";
insertstr(dst, src, sizeof(dst), 3);  // result should be "fooxxxbar"


 Line | dst            | src           | size | pos | sc | dc | i  | j  | Description
------+----------------+---------------+------+-----+----+----+----+----+
   1  | "foobar\0????" | "xxx\0??????" |  20  |  3  | ?  | ?  | ?  | ?  | Call function
   3  | "foobar\0????" | "xxx\0??????" |  20  |  3  | 2  | 5  | 7  | ?  | Declare sc and set it to one less than number of chars in src, similar for dc and dst, set i to the sum of dc and sc (two less than the combined length)
   5  | "foobar\0????" | "xxx\0??????" |  20  |  3  | 2  | 5  | 7  | ?  | i does not exceed size, go to line 11
  11  | "foobar\0????" | "xxx\0??????" |  20  |  3  | 2  | 5  | 7  | 5  | Declare j and set it equal to dc
  12  | "foobar\0????" | .."x\0??????" |  20  |  3  | 2  | 5  | 7  | 5  | Set src to point to the last line of it's contents
  14  | "foobar\0????" | .."x\0??????" |  20  |  3  | 2  | 5  | 7  | 5  | j > pos is true, go to line 16
  16  | "foobar\0r???" | .."x\0??????" |  20  |  3  | 2  | 5  | 7  | 5  | Copy the last letter of dst to later in the string, to make room for src
  17  | "foobax\0r???" | .."x\0??????" |  20  |  3  | 2  | 5  | 7  | 5  | Copy the last letter of src to it's new position in dst
  18  | "foobax\0r???" | ."xx\0??????" |  20  |  3  | 2  | 5  | 6  | 4  | Decrement i, j and src, go to line 14
  14  | "foobax\0r???" | ."xx\0??????" |  20  |  3  | 2  | 5  | 6  | 4  | j > pos is true, go to line 16
  16  |
...

I guess you can take something really simple and make it obnoxiously complex. And I guess that you "see no other way" to structure you code, as usual.

My question is, why do you keep asking questions when you never listen to advice?

But I'll try anyway. Why don't you use the standard string library to do this in a few lines of code, like any reasonable programmer would?

As for the debugger part I haven't tried the actual function yet, was going to do so 2mw after I'm done with my obligations, figured it couldn't hurt to post the code and see if any1 spots a problem before then.

This forum doesn't require you to squeeze your messages into a SMS/Twitter size limit. Never sacrifice clarity (I seriously don't know what 2mw is, never seen it before) to save a few keystrokes. That goes for your code too. Descriptive names are better. sc and dc are questionable. i and j are generally okay, but in this case could perhaps be better named as length_after_insert and dst_tail_copy_position.

How about finishing out that table and seeing what you come up with? That table/tracing method is basically you pretending to be the computer/debugger. You can do this now, before you "try the actual function". You don't even need a computer, it can be done with paper and pencil.

@awsdert:
Jimmy isn't necessarily saying there is the exact function you need already in there. He is saying there are functions that might facilitate you writing this function and help you from making errors. Also, he may be saying you should read in more detail, exactly what the functions you are using do/return. You may be using one incorrectly.

Seriously, finish out that table (you don't have to do it on the computer and post it back). It should help you pinpoint the error(s).

@Jimmy/awsdert:
puts is not vulnerable to format string attacks since it doesn't use format specifiers. The potential for format string attacks comes from

Code:

printf(line);

Just to be sure I tried your foobar example and got the expected fooxxxbar

Originally Posted by anduril462

Puts is not vulnerable to format string attacks since it doesn't use format specifiers. The potential for format string attacks comes from

Code:

printf(line);

I'm actually aware of that.

During my work as code auditor we ran a lot of metrics on a lot of source code. People that use puts are the ones that also does:

Code:

printf(line);

I'm not saying that only people that use puts does that, but statistically they are the ones that do so. My guess is that it is due to habits.

Habit is also the reason they can use source code to profile who wrote code with 97 % success, given a training set of known source and a piece of unidentified source code.

Well I wasn't aware of it so it will be a good lesson to remember regardless of which version of printf I use.

Originally Posted by awsdert

Well I wasn't aware of it so it will be a good lesson to remember regardless of which version of printf I use.

Well, anduril462 was correct in saying what he said and I was pretty unclear with what I meant, so it was a good correction from him.

Hi,

what anduril was reffering to is to use the standard string.h functions of the clib, like:

Code:

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

char * str_insert(char * insert, char * src, size_t offset)
{
    char * temp;
    if ((temp = malloc(strlen(src) + strlen(insert) + 1)) != NULL) {
        strncpy(temp, src, offset);
        strcat(temp, insert);
        strcat(temp, &src[offset]);
    }
    return temp;
}


int main(int argc, char ** argv)
{
    char * newstr;
    int offset;

    if (argc < 4) {
        printf("Missing parameters.\n");
        printf("Syntax: %s <string> <string_to_insert> <offset>\n", argv[0]);
    } else {
        offset = (int) strtol(argv[3], NULL, 10);
        if ( (newstr = str_insert(argv[2], argv[1], offset)) != NULL ) {
            printf("The new string is: %s\n", newstr);
        }
    }
    return 0;
}

which gives output like this:

Code:

 
me@Sirius:/data/secure/source/cprogs$ ./strins_ex abcdef xyz 1
The new string is: axyzbcdef

me@Sirius:/data/secure/source/cprogs$ ./strins_ex abcdef xyz 4
The new string is: abcdxyzef

me@Sirius:/data/secure/source/cprogs$ ./strins_ex abcdef xyz 10
The new string is: abcdefxyz

here is a link to https://gnu.org/software/libc/manual...mono/libc.html
the standard gnu c library.

Cheers,