How to fix this SEGFAULT

Pow(x,2) is slightly slower than x*x and RNGs need to be fast for practical use.
converting numbers to and from strings is very slow.
push_back can be slow if you did not preallocate the vector.
not sure if the recursive calls here are going to optimize correctly or not.

I highly recommend you start over with a simply numeric manipulation algorithm that avoids the string stuff. No vector, none of that stuff. Just a small number of integer variables and some code is sufficient. While the linear-congruentials are terrible, you can use one (or 2 of them) as a starting point, throw in some trig and bit flips (xor, or endian reverse, etc) and the like. Test it, see if its random (get a distribution of a few million numbers from it).

That aside, do you know exactly where it crashes? Does it crash for all or most input? Any chance you blew the stack (too many recursive calls before resolution of the base case will crash a program)?

Last edited on

@jonnin Thanks you for your recommendations! Well, actually the reason I use these convertions is because I want to delete some particular digits. Unfortunately it crushes regardless of the input. The only exception is when p is 0. I don't think I broke the stack due to the amount of recursive calls, since the amount of recursive calls is 1, every time I test it. As I said, the problem probably occurs due to misuse of std::stoi or std::string::erase. The seed (s) MUST be 4 bytes long.

EDIT:

The error occurs when I convert b to an interger using std::stoi. It throws an out_of_range error but even when I use std::stoul or even std::stoull but the error still persist.

Last edited on

Ok, so that is a great place to start.
can you isolate the input to stoi? Make sure it does not have leading whitespace or some issue etc?

does 0004 count as 4 bytes long?
Or does seed have to be > 999 or < -999 ?

Last edited on

doug4 (1538)

It sounds like you are making progress on the seg fault. I have a couple other things I noticed.

What I wonder about is lines 7 - 11. On the surface it looks like you are trying to get rid of all '0's from the base 10 representation of the number (1024 -> 124). However, if there are 2 consecutive '0's, you will only get rid of the 1st. (100 -> 10). If this is what you are aiming for, great. But if not, you need to do something inside your loop when you erase a character from the string (maybe decrement the counter to point to the last character checked).

Also, line 19 doesn't do anything. You are working on a string that was originally 7 characters long. You deleted the 1st 2 characters, so the string is now 5 characters long. b.begin() + 5 is now pointing to b.end(), so the erase does nothing. Did you mean to use b.begin + 4? That would give you a 4-digit number pushed onto the vector, the same as b.size() == 8, 6, 5 and 4.

I actually get an std::invalid_argument on line 24 when b is ".000" which of course is not a valid int.

Thanks a lot for your time :) .

@jonnin The input passed into the std::stoi looks fine since theres not any leading whitespace or anything like that.
No it doesn't. The seed has to be an interger in [1000, 9999] . Note that every variable is unsigned.

@doug4 Well, it seems I'm getting somewhere. Yeah, I want to delete every single 0, but I really can't get why it doesn't work with consecutive 0's. Wow you're really sharp thanks a lot for pointing that out. ^^

@Thomas1965 I see, yes you're right but as I said, b must be 4 bytes long also contains an unsinged interger.

Guys, I have a question, does std::string::erase() really erases characters or it replaces them with something else? If std::string::erase() doesn't erase characters, then this is why it doesn't work. It seems really strange that std::stoi throws an exception...

Last edited on

It seems really strange that std::stoi throws an exception...

Have a look here: http://www.cplusplus.com/reference/string/stoi/

b must be 4 bytes long also it's an unsigned integer.

No it isn't unsigned integer , pow returns a double so b contains a double in string form

BTW Could you tell us what fpR, s and p stand for. Also a short description what the function should do would be helpful. Do you maybe want a number with 4 digits and without zeroes ?

Last edited on

^^^^ X*X I said that :)

Do you maybe want a number with 4 digits and without zeroes ?

sounds like a job for KISS (the C way, or C++ equiv code)

char derp[] = "123456789";
char num[5] = {0};
for(i = 0; i < 5; i++)
num[i] = derp[rand()%strlen(derp)];
x = atoi(num);

Even that is a little loose, it could be simplified but I'd need more time to think. The strlen can be done once and only once, for sure. I think you can do it with digits instead of numbers, and multiply by 10, and avoid the strings entirely.

so really...

char derp[] = {1,2,3,4,5,6,7,8,9,0}; //hackity... the 0 here is end of string, not used.
int num = 0;
int p = 1;
int len = strlen(derp);
for(i = 0; i < 5; i++, p*=10)
num += derp[rand()%len] * p;

Last edited on

@Thomas1965

"If no conversion could be performed, an invalid_argument exception is thrown."

It's strange because it seems that the conversion can happen.

No it isn't unsigned integer , pow returns a double so b contains a double in string form

I forgot to mention that I made a change to the code. Now I'm not using pow at all. Instead I'm using s*s. I'm going to post the updated code ASAP.

Ofc. fpR I don't quite remember why I gave that name to this function. But it's just a recursive function that generates numbers that seemingly have no connection between them and pushes them into a vector with each permutation.

s stands for seed, it's a 4 byte interger.
p stands for remaining permutations, when it reaches 0 it means that the function must stop.

Yes I'm trying to generate numbers with 4 digits with no zeroes.

Firstly I square the seed, then I delete all the zeroes, then I delete characters from it to generate a 4 digit-long interger, then I reduce the permutations remaing by one and do the same thing until p = 0.

Last edited on

Yes I'm trying to generate numbers with 4 digits with no zeroes.

If that is ALL you want, the last segment I gave you would do it quite efficiently, or similar C++ code. Assuming I didnt mess it up, it should be close and the concept is there.

Last edited on

doug4 (1538)

but I really can't get why it doesn't work with consecutive 0's.

    for (unsigned int i = 0; i < b.size(); i++){
        if (b[i] == '0'){
            b.erase(i, 1);
        }
    }

Let's say you start with the string b = "1002".

i = 0: b[i] = '1' --> keep going
i = 1: b[i] = '0' --> erase the character. b is now "102". b.size() is now 3. 'i' is still 1.
i = 2: b[i] = '2' (the string was changed) --> keep going
i = 3: i is now greater than b.size(), so loop exits.

Result: b = "102".

@jonnin Hahahaha, see? I followed your suggestion :) . Unfortunately, I don't want to use rand() because I want to be able to know what the next number is going to be knowing the first seed.

Why don't you try a simple approach:

int random()
{
  int retval = 0;

  for (int i = 0; i < 4; i++)
  {
    int digit = 1 + (rand() % 8);
    retval *= 10;
    retval += digit;
    
  }
  return retval;
}

On a simple test run I got the following numbers:

Not sure how random they must be.

@Thomas1965 As I said I must be able to predict those numbers by knowing just the seed. So unfortunately I can't use rand() because it relies on variables like time etc. If you run the code you provided multiple times, each time the result will be different. But I want the result to be the same.

But I want the result to be the same.

Sorry , somehow I missed it.

@Thomas1995 It's ok.

@doug4 Ah now I get it I'll fix it . Thanks a lot ^^

I think that std::string::erase might be the problem, is there anything else I can use instead?

erase should work just fine if you give it the correct parameters.

You have to know exactly what you are erasing, how long it is, etc. You could do it by hand but its the same problem with more code, you still have to know the bounds and take care with it.

Instead of erase you can build the string from the individual digits:

string noZeroDigits(unsigned int num)
{
  string digits;

  if (num == 0)
    return digits;

  do
  {
    if (num % 10 != 0)
      digits += (num % 10 + '0');

    num /= 10;

  }while(num > 0);

  return string(digits.rbegin(), digits.rend());
}