-3

I found this c++ implementation of SHA1 on this page zedwood.com. Why doesn't the this function work. It says that max has to be a constant. How to get around this problem?

void SHA1::read(std::istream &is, std::string &s, int max)
{
char sbuf[max];
is.read(sbuf, max);
s.assign(sbuf, is.gcount());
}``
Matej Dražić
  • 53
  • 1
  • 1
  • 5
  • 2
    Does this answer your question? [c++ expression must have a constant value](https://stackoverflow.com/questions/33288431/c-expression-must-have-a-constant-value) or [expression must have a constant value in c++](https://stackoverflow.com/questions/14430994/expression-must-have-a-constant-value-in-c) – walnut Jan 13 '20 at 14:01
  • 1
    Basically the code on zedwood.com isn't standard C++ (read: bad). But it will probably work on GCC because it supports VLAs as an extension. – rustyx Jan 13 '20 at 14:11

2 Answers2

2

It's what it says. max is a runtime variable. You can't use those for array bounds in C++.

I don't know why the tutorial author did that, except that some compilers accept it as an extension, and that some tutorials are bad.

Consider using a std::vector<char> sbuf(max) instead, or skip the dynamic allocation and use a char sbuf[BLOCK_BYTES] — I don't really understand why the buffer must be constrained to max. It doesn't matter if you don't completely fill it.

Lightness Races in Orbit
  • 378,754
  • 76
  • 643
  • 1,055
0

Standard C++ doesn't allow the creation of arrays with a size based on a variable.

char sbuf[max];

max has to be a value known at compile time. Some compilers however might allow this. So to get around you problem either use a compiler that supports dynamic array sizes or you need to dynamically allocate your array:

char* sbuf = new char[max];

You will have to manually delete your array by calling delete[] sbuf or using a smart pointer instead.

Eric
  • 1,183
  • 6
  • 17
  • At the very least mention that you need to `delete[]` the array when using `new`. But even then using `new` is bad advice when `std::vector` also does the job, especially considering that there may be exceptions thrown in OP's code. – walnut Jan 13 '20 at 14:03
  • Don't use `new`. Use containers. And let's not use terms like "heap" – Lightness Races in Orbit Jan 13 '20 at 14:03
  • [Pro Tip] Don't use `new`. `new` is evil and must be stopped ;) – NathanOliver Jan 13 '20 at 14:03
  • *So to get around you problem either use a compiler that supports dynamic array sizes* -- Why do that when there is a solution that works for all compilers? – PaulMcKenzie Jan 13 '20 at 14:04
  • 1
    @PaulMcKenzie The question simply was why this code (that he didn't write himself) isn't compiling. Especially in the context of this code coming from a library that is expected to work you have to point out that ther are some compilers that support this even tho it isn't standard c++. Also changing the code of a thirdparty library might not be the best idea. If this library is writting for a compiler that supports dynamic array sizes it's not a stupid idea to consider using the compiler the library was written for instead of altering the library code. – Eric Jan 13 '20 at 14:19
  • @LightnessRacesBY-SA3.0 What's wrong with the term heap? – Eric Jan 13 '20 at 14:39
  • @Eric It's an archaic term from the days when C programs were hand-cranked onto slate, that refers to some specific data structure in memory on some particular implementation/platform, completely ignoring the fact that C++ is an abstraction and your compiler and computer are highly complex and even your dynamically allocated objects may not be on any "heap". Let's talk about storage duration instead and get rid of these old, misleading "stack" vs "heap" terms. – Lightness Races in Orbit Jan 13 '20 at 14:43