Return to Answer

It's good overall, and for a toy or class program this is adequate, but it's been a while since I've done a review, so it's time to get super picky :).

There is a very major problem in that the IO handling is wrong. Try firing up your program, and instead of a valid number of items to sum, put in something non-numeric. It will infinite loop.

In short, scanf doesn't move forward in a stream when it doesn't consume anything¹. You'll need to either read a line at a time (fgets) and parse an integer out of the line, or you'll need to consume the rest of the line and try again (the easiest approach to that would be to loop on getline until you get a \n).

In a similar vein, your input reading in the loop to collect inputs is just wrong. All it takes is a single incorrect input and the program falls apart.

¹ It might be worth noting at this point that scanf has other problems as well.

There's also some portability issues. Before C99, variable length arrays are not required to be supported by the standard, and after C99, they're standardized but not required. This means that C99 is the only standard that requires VLA. That's a pretty precarious situation to put yourself in.

In case you're not sure what I'm talking about, arr is the variable length array since num is not a compile time constant (there's magic going on under the hood -- it's not a normal array, the compile is just hiding that).

(Realistically, every compile you're going to come across is going to support this. Just one of the many gotchas in C with regards to portability if you're ever going to experience a truly rare environment.)

I don't understand why the array and sum are unsigned. Seems the same functionality would work just as well if you were to allow signed numbers? The only reason I can think of for using unsigned values is if you're trying to eek out a little more range, but that's probably misguided and if a 64 bit integer isn't large enough, you're probably into arbitrary precision land.

In short, I think all of your unsigned long long stuff should just be long long int or int64_t (from stdint.h). Even though long long int is actually guaranteed to be 64 bits or more whereas int64_t is required to be exactly 64 bits (and is not required to be supported), I think int64_t is clearer to read (and realistically, if long long exists, int64_t is going to exist).

This is mildly controversial, but I'm not really a fan of short unless you're doing it because you actually need a specific size (memory constrained stuff, you're going to have a lot of them, alignment issues, etc). ints come across a bit more naturally to work with, and depending on CPU architecture, a short is probably being treated as an int anyway.

It's good overall, and for a toy or class program this is adequate, but it's been a while since I've done a review, so it's time to get super picky :).

There is a very major problem in that the IO handling is wrong. Try firing up your program, and instead of a valid number of items to sum, put in something non-numeric. It will infinite loop.

In short, scanf doesn't move forward in a stream when it doesn't consume anything¹. You'll need to either read a line at a time (fgets) and parse an integer out of the line, or you'll need to consume the rest of the line and try again (the easiest approach to that would be to loop on getline until you get a \n).

In a similar vein, your input reading in the loop to collect inputs is just wrong. All it takes is a single incorrect input and the program falls apart.

¹ It might be worth noting at this point that scanf has other problems as well.

There's also some portability issues. Before C99, variable length arrays are not required to be supported by the standard, and after C99, they're standardized but not required. This means that C99 is the only standard that requires VLA. That's a pretty precarious situation to put yourself in.

In case you're not sure what I'm talking about, arr is the variable length array since num is not a compile-time constant (there's magic going on under the hood — it's not a normal array, the compiler is just hiding that).

(Realistically, every compiler you're going to come across is going to support this. Just one of the many gotchas in C with regards to portability if you're ever going to experience a truly rare environment.)

I don't understand why the array and sum are unsigned. Seems the same functionality would work just as well if you were to allow signed numbers? The only reason I can think of for using unsigned values is if you're trying to eke out a little more range, but that's probably misguided and if a 64 bit integer isn't large enough, you're probably into arbitrary precision land.

In short, I think all of your unsigned long long stuff should just be long long int or int64_t (from stdint.h). Even though long long int is actually guaranteed to be 64 bits or more whereas int64_t is required to be exactly 64 bits (and is not required to be supported), I think int64_t is clearer to read (and realistically, if long long exists, int64_t is going to exist).

This is mildly controversial, but I'm not really a fan of short unless you're doing it because you actually need a specific size (memory constrained stuff, you're going to have a lot of them, alignment issues, etc). ints come across a bit more naturally to work with, and depending on CPU architecture, a short is probably being treated as an int anyway.

It's good overall, and for a toy or class program this is adequate, but it's been a while since I've done a review, so it's time to get super picky :).

There is a very major problem in that the IO handling is wrong. Try firing up your program, and instead of a valid number of items to sum, put in something non-numeric. It will infinite loop.

In short, scanf doesn't move forward in a stream when it doesn't consume anything. You'll need to either read a line at a time (fgets) and parse an integer out of the line, or you'll need to consume the rest of the line and try again (the easiest approach to that would be to loop on getline until you get a \n).

In a similar vein, your input reading in the loop to collect inputs is just wrong. All it takes is a single incorrect input and the program falls apart.

There's also some portability issues. Before C99, variable length arrays are not required to be supported by the standard, and after C99, they're standardized but not required. This means that C99 is the only standard that requires VLA. That's a pretty precarious situation to put yourself in.

In case you're not sure what I'm talking about, arr is the variable length array since num is not a compile time constant (there's magic going on under the hood -- it's not a normal array, the compile is just hiding that).

(Realistically, every compile you're going to come across is going to support this. Just one of the many gotchas in C with regards to portability if you're ever going to experience a truly rare environment.)

I don't understand why the array and sum are unsigned. Seems the same functionality would work just as well if you were to allow signed numbers? The only reason I can think of for using unsigned values is if you're trying to eek out a little more range, but that's probably misguided and if a 64 bit integer isn't large enough, you're probably into arbitrary precision land.

In short, I think all of your unsigned long long stuff should just be long long int or int64_t (from stdint.h). Even though long long int is actually guaranteed to be 64 bits or more whereas int64_t is required to be exactly 64 bits (and is not required to be supported), I think int64_t is clearer to read (and realistically, if long long exists, int64_t is going to exist).

I also think there's no reason that num is a short. I'd just make it an int and be done with, or even an int32_t or int64_t.

while(num <= 0 || num >= 256)

num > 256 can never be true (well, at least not on systems with 16 bit shorts). If you really want to check for overflow, you're either going to have to handle IO differently, or you're going to need to go up a size large in integer type and check for it.

It's good overall, and for a toy or class program this is adequate, but it's been a while since I've done a review, so it's time to get super picky :).

There is a very major problem in that the IO handling is wrong. Try firing up your program, and instead of a valid number of items to sum, put in something non-numeric. It will infinite loop.

In short, scanf doesn't move forward in a stream when it doesn't consume anything¹. You'll need to either read a line at a time (fgets) and parse an integer out of the line, or you'll need to consume the rest of the line and try again (the easiest approach to that would be to loop on getline until you get a \n).

In a similar vein, your input reading in the loop to collect inputs is just wrong. All it takes is a single incorrect input and the program falls apart.

¹ It might be worth noting at this point that scanf has other problems as well.

There's also some portability issues. Before C99, variable length arrays are not required to be supported by the standard, and after C99, they're standardized but not required. This means that C99 is the only standard that requires VLA. That's a pretty precarious situation to put yourself in.

In case you're not sure what I'm talking about, arr is the variable length array since num is not a compile time constant (there's magic going on under the hood -- it's not a normal array, the compile is just hiding that).

(Realistically, every compile you're going to come across is going to support this. Just one of the many gotchas in C with regards to portability if you're ever going to experience a truly rare environment.)

I don't understand why the array and sum are unsigned. Seems the same functionality would work just as well if you were to allow signed numbers? The only reason I can think of for using unsigned values is if you're trying to eek out a little more range, but that's probably misguided and if a 64 bit integer isn't large enough, you're probably into arbitrary precision land.

In short, I think all of your unsigned long long stuff should just be long long int or int64_t (from stdint.h). Even though long long int is actually guaranteed to be 64 bits or more whereas int64_t is required to be exactly 64 bits (and is not required to be supported), I think int64_t is clearer to read (and realistically, if long long exists, int64_t is going to exist).

This is mildly controversial, but I'm not really a fan of short unless you're doing it because you actually need a specific size (memory constrained stuff, you're going to have a lot of them, alignment issues, etc). ints come across a bit more naturally to work with, and depending on CPU architecture, a short is probably being treated as an int anyway.