Skip to main content
Code Review

Return to Answer

Commonmark migration
Source Link

###Input checking###

Input checking

  1. If the caller passes in a base greater than 17, you will read past the end of rep.
  2. If the caller passes in a base of 0, you will divide by zero.
  3. If the caller passes in a base of 1, your code will infinite loop.
  4. If the caller passes in a negative value but the base is not 10, you will read past the front of rep because your modulus will be negative. You should always make value positive before proceeding with your loop.

###Insufficient buffer size###

Insufficient buffer size

On a system with 64-bit ints and a base of 2, you will need a buffer of size 65 instead of 50. (Or 66 if you allow negative values for all bases).

###Code simplification###

Code simplification

This code:

if (value == 0)
 *--ptr = rep[value % base];
while (value != 0)
{
 *--ptr = rep[value % base];
 value /= base;
}

could be simplified to:

do {
 *--ptr = rep[value % base];
 value /= base;
} while (value != 0);

I'm not sure from your question whether you were allowed to use a do loop or not. Also, your question says you are supposed to return an allocated buffer but you intentionally didn't, so I'm not sure how to review that part of it.

###Input checking###

  1. If the caller passes in a base greater than 17, you will read past the end of rep.
  2. If the caller passes in a base of 0, you will divide by zero.
  3. If the caller passes in a base of 1, your code will infinite loop.
  4. If the caller passes in a negative value but the base is not 10, you will read past the front of rep because your modulus will be negative. You should always make value positive before proceeding with your loop.

###Insufficient buffer size###

On a system with 64-bit ints and a base of 2, you will need a buffer of size 65 instead of 50. (Or 66 if you allow negative values for all bases).

###Code simplification###

This code:

if (value == 0)
 *--ptr = rep[value % base];
while (value != 0)
{
 *--ptr = rep[value % base];
 value /= base;
}

could be simplified to:

do {
 *--ptr = rep[value % base];
 value /= base;
} while (value != 0);

I'm not sure from your question whether you were allowed to use a do loop or not. Also, your question says you are supposed to return an allocated buffer but you intentionally didn't, so I'm not sure how to review that part of it.

Input checking

  1. If the caller passes in a base greater than 17, you will read past the end of rep.
  2. If the caller passes in a base of 0, you will divide by zero.
  3. If the caller passes in a base of 1, your code will infinite loop.
  4. If the caller passes in a negative value but the base is not 10, you will read past the front of rep because your modulus will be negative. You should always make value positive before proceeding with your loop.

Insufficient buffer size

On a system with 64-bit ints and a base of 2, you will need a buffer of size 65 instead of 50. (Or 66 if you allow negative values for all bases).

Code simplification

This code:

if (value == 0)
 *--ptr = rep[value % base];
while (value != 0)
{
 *--ptr = rep[value % base];
 value /= base;
}

could be simplified to:

do {
 *--ptr = rep[value % base];
 value /= base;
} while (value != 0);

I'm not sure from your question whether you were allowed to use a do loop or not. Also, your question says you are supposed to return an allocated buffer but you intentionally didn't, so I'm not sure how to review that part of it.

added 53 characters in body
Source Link
JS1
  • 28.8k
  • 3
  • 41
  • 83

###Input checking###

  1. If the caller passes in a base greater than 17, you will read past the end of rep.
  2. If the caller passes in a base of 0, you will divide by zero.
  3. If the caller passes in a base of 1, your code will infinite loop.
  4. If the caller passes in a negative value but the base is not 10, you will read past the front of rep because your modulus will be negative. You should always make value positive before proceeding with your loop.

###Insufficient buffer size###

On a system with 64-bit ints and a base of 2, you will need a buffer of size 65 instead of 50. (Or 66 if you allow negative values for all bases).

###Code simplification###

This code:

if (value == 0)
 *--ptr = rep[value % base];
while (value != 0)
{
 *--ptr = rep[value % base];
 value /= base;
}

could be simplified to:

do {
 *--ptr = rep[value % base];
 value /= base;
} while (value != 0);

I'm not sure from your question whether you were allowed to use a do loop or not. Also, your question says you are supposed to return an allocated buffer but you intentionally didn't, so I'm not sure how to review that part of it.

###Input checking###

  1. If the caller passes in a base greater than 17, you will read past the end of rep.
  2. If the caller passes in a base of 0, you will divide by zero.
  3. If the caller passes in a base of 1, your code will infinite loop.
  4. If the caller passes in a negative value but the base is not 10, you will read past the front of rep because your modulus will be negative. You should always make value positive before proceeding with your loop.

###Insufficient buffer size###

On a system with 64-bit ints and a base of 2, you will need a buffer of size 65 instead of 50.

###Code simplification###

This code:

if (value == 0)
 *--ptr = rep[value % base];
while (value != 0)
{
 *--ptr = rep[value % base];
 value /= base;
}

could be simplified to:

do {
 *--ptr = rep[value % base];
 value /= base;
} while (value != 0);

I'm not sure from your question whether you were allowed to use a do loop or not. Also, your question says you are supposed to return an allocated buffer but you intentionally didn't, so I'm not sure how to review that part of it.

###Input checking###

  1. If the caller passes in a base greater than 17, you will read past the end of rep.
  2. If the caller passes in a base of 0, you will divide by zero.
  3. If the caller passes in a base of 1, your code will infinite loop.
  4. If the caller passes in a negative value but the base is not 10, you will read past the front of rep because your modulus will be negative. You should always make value positive before proceeding with your loop.

###Insufficient buffer size###

On a system with 64-bit ints and a base of 2, you will need a buffer of size 65 instead of 50. (Or 66 if you allow negative values for all bases).

###Code simplification###

This code:

if (value == 0)
 *--ptr = rep[value % base];
while (value != 0)
{
 *--ptr = rep[value % base];
 value /= base;
}

could be simplified to:

do {
 *--ptr = rep[value % base];
 value /= base;
} while (value != 0);

I'm not sure from your question whether you were allowed to use a do loop or not. Also, your question says you are supposed to return an allocated buffer but you intentionally didn't, so I'm not sure how to review that part of it.

added 152 characters in body
Source Link
JS1
  • 28.8k
  • 3
  • 41
  • 83

###Input checking###

  1. If the caller passes in a base greater than 17, you will read past the end of rep.
  2. If the caller passes in a base of 0, you will divide by zero.
  3. If the caller passes in a basebase of 1, your code will infinite loop.
  4. If the caller passes in a negative value but the base is not 10, you will read past the front of rep because your modulus will be negative. You should always make value positive before proceeding with your loop.

###Insufficient buffer size###

On a system with 64-bit ints and a base of 2, you will need a buffer of size 65 instead of 50.

###Code simplification###

This code:

if (value == 0)
 *--ptr = rep[value % base];
while (value != 0)
{
 *--ptr = rep[value % base];
 value /= base;
}

could be simplified to:

do {
 *--ptr = rep[value % base];
 value /= base;
} while (value != 0);

I'm not sure from your question whether you were allowed to use a do loop or not. Also, your question says you are supposed to return an allocated buffer but you intentionally didn't, so I'm not sure how to review that part of it.

###Input checking###

  1. If the caller passes in a base greater than 17, you will read past the end of rep.
  2. If the caller passes in a base of 0, you will divide by zero.
  3. If the caller passes in a base of 1, your code will infinite loop.
  4. If the caller passes in a negative value but the base is not 10, you will read past the front of rep because your modulus will be negative. You should always make value positive before proceeding with your loop.

###Insufficient buffer size###

On a system with 64-bit ints and a base of 2, you will need a buffer of size 65 instead of 50.

###Code simplification###

This code:

if (value == 0)
 *--ptr = rep[value % base];
while (value != 0)
{
 *--ptr = rep[value % base];
 value /= base;
}

could be simplified to:

do {
 *--ptr = rep[value % base];
 value /= base;
} while (value != 0);

I'm not sure from your question whether you were allowed to use a do loop or not.

###Input checking###

  1. If the caller passes in a base greater than 17, you will read past the end of rep.
  2. If the caller passes in a base of 0, you will divide by zero.
  3. If the caller passes in a base of 1, your code will infinite loop.
  4. If the caller passes in a negative value but the base is not 10, you will read past the front of rep because your modulus will be negative. You should always make value positive before proceeding with your loop.

###Insufficient buffer size###

On a system with 64-bit ints and a base of 2, you will need a buffer of size 65 instead of 50.

###Code simplification###

This code:

if (value == 0)
 *--ptr = rep[value % base];
while (value != 0)
{
 *--ptr = rep[value % base];
 value /= base;
}

could be simplified to:

do {
 *--ptr = rep[value % base];
 value /= base;
} while (value != 0);

I'm not sure from your question whether you were allowed to use a do loop or not. Also, your question says you are supposed to return an allocated buffer but you intentionally didn't, so I'm not sure how to review that part of it.

Source Link
JS1
  • 28.8k
  • 3
  • 41
  • 83
Loading
lang-c

AltStyle によって変換されたページ (->オリジナル) /