[vox-tech] C-Newbie needs help with a source-code

[Micah J. Cowan]

Unlike some other respondants, I have no problem with the size of your
code: it's pretty average for what we get over at comp.lang.c (where
you might consider posting for even more help).

What I do have a problem with is that you didn't adequately explain
what you are trying to do, and I can't read German comments :-)

However, I think you're trying to do this:

 1. Read in an int
 2. Determine how many digits would be required to store a
 binary-representation of int as a series of ints, one per bit.
 3. Create an array large enough to do so, and fill it in with that
 binary representation.
 4. Print that array out as a string of binary digits.

On Thu, Oct 30, 2003 at 03:06:03PM +0100, danielweyh@freenet.de wrote:
> /////////////////////////////////////////////////////////////////////
> /* Mit diesem Programme möchte ich
> Dezimalzahlen in Zahlen des Binären
> Systems umwandeln und diese anschließend
> ausgeben */
> 
> #include <stdio.h>
> 
> main () {

In both C90 and C99, main() returns an int. In C90, the above is legal
as the return type of a function defaults to int if you don't
explicitly say what it is; but in C99 implicit return types have been
removed.

I recommend:

  int main (void) {

It's important to realize that in C this is *not* the same thing as

  int main () {

although in C++ they have the same meaning. In C, the latter is not a
prototype, and the empty parentheses doesn't mean that main() doesn't
take any parameters; it means that your declaration doesn't specify
what parameters it accepts. In particular, if you were to
(recursively) call main() within your program with a parameter (of any
arbitrary type), it would probably be accepted without complaint by the
compiler as legal code; whereas in the former case, a compiler is
*required* to speak up about it.

>     int zahl;     ///
>     int bit = 0;  /// Variablendeklaration der "zahl", des "bit" und der Zählvariable "i"
>     int i = 0;    ///
>     int a = 0;

You never use the int a in your code.

The // comments do not exist in C90; if you intend to write code that
compiles in C90, you should change them to /* */ comments. Otherwise,
you should note that only two /s are required to start a line comment.

>     fprintf(stderr,"Bitte geben sie eine positive Zahl ein, welche dann in das duale Zahlensystem umgewandelt werden soll:\n");             /// Ausgabe

This is just stylistic, but I don't personally like such long
lines. gcc supports breaking (newlines) within strings, but that
feature is deprecated (and unnecessary). The preprocessor
automatically concatenates string literal tokens which are adjacent,
so you could do:

    fprintf(stderr,"Bitte geben sie eine positive Zahl ein, welche "
            "dann in das duale Zahlensystem umgewandelt "
            "werden soll:\n");   // Ausgabe

>     scanf("%d", &zahl); /// Einlesen auf Adresse &zahl

If you're going to use scanf(), you really should check the return
value to make sure that the conversion succeeded. What if the user
types ".", or "abc{}"?

>     if(zahl > 2147483646){

This is not a good way to check for overflow. For one thing, there is
no guarantee that you're dealing with a 32-bit int type (and even if
you are, according to the Standard, not all bits in an int type need
to contribute to the value [padding bits], though in practice such
ints have not existed and are not likely to). Some systems define int
as 16 bits, some 64. Some older systems had ints that were not made up
of octets, since their natural byte size was not 8 bits.

A more portable (but still inadvisable) comparison would be:

  if (zahl == INT_MAX)

(after #including <limits.h>).

But in actuality, the behavior of scanf() is undefined if the
converted input sequence is out of range for the type of the object in
which it is expected to be stored. In practice, this means it's quite
possible that an overflow could result in a *negative* number (causing
your test to fail), or it could raise a signal. Luckily, glibc will
truncate it to INT_MAX; but you can't rely on this behavior.

There's also the fact that if you actually enter 2147483647, which is
representable in a typical int, you'll get a false positive in your
test, even though it's a valid value.

I'd recommend that instead of scanf(), you use fgets() combined with
strtol(). strtol() allows you to distinguish between whether a valid
value at the extreme of representable values was entered, or whether
an overflowed, because in the latter case it sets errno to ERANGE
(make sure and clear errno just before the call to strtol(), just in
case it was set silently by some internal workings of the library).

> 	printf("Die Zahl ist zu groß!!!\n"); /// aussortieren zu großer Zahlen
>     }else{
> 	if(zahl < 0){
> 	    printf("Ich sagte doch POSITIVE Zahl...\n"); /// aussortieren negativer Zahlen
> 	}else{
> 
> 	    int lange = bestimme_laenge(zahl);  

You are calling bestimme_laenge(), but you haven't declared it
yet. This is illegal in C99, and even in C90, it is problematic (the
implied declaration has no prototype, and won't protect you from
mistakes in how you've called it). Either place a declaration of
bestimme_laenge() above main(), or move the whole declaration up
there.

> /// Initialisierung und Belegung von laenge mit dem Rückgabewert der Funktion "bestimme_laenge", der die Variable "zahl" übergeben wird
> 	    int Dec[lange-1];  /// Initialisierung eines Array mit der Länge lange bzw. bis zur Stelle lange-1

The above declaration of Dec is called a VLA (variable-length array),
since its size is determined at runtime, and can be different each
time the declaration is encountered. This is perfectly acceptable in
C99, but not in C90. If you want to be portable to C90, you'll need to
use malloc() and free().

Also, lange - 1 is *not* enough digits to represent the bit
stream. You need lange digits. Consider some test cases ("1", for
example), and see what lange amounts to.

Oh yeah: and Dec seems to be a misnomer for a variable that stores a
binary representation... ;-)

> 	    printf("Die Zahl %d im dualen System ausgedrückt heißt:\n", zahl);
> 
> 	    while (zahl > 0){
> 		bit = zahl % 2;
> 		zahl = (zahl - bit) / 2;
> 		Dec[i] = bit;
> 		i++;
> 	    }

Storing the bits in reverse order. Since you already know how many
digits, you could pretty easily store it in order. Also, you don't
consider the case for the input being "0" in the first place. Your
program would produce an empty binary string. Is that what you want?

Also, I'm not sure what purpose subtracting bit from zahl is supposed
to do, but it won't have any effect on the results, because the only
time bit is not zero is when zahl was odd; and for any odd integer
"o", (o / 2 == (o - 1) / 2), since the extra 1 in math for real
numbers would become 0.5 in the result, which would be simply
truncated in the integer divide you are doing. So just do:

  zahl /= 2;

And you're fine.

> 	    for(i = lange ; i >= 0; i--){

This leads to an invalid memory access. Dec doesn't have an element
[lange]; until you fix the declaration, it doesn't even have an elemnt
[lange - 1] (remember the first element is [0]).

The fact that Dec is declared too short, combined with the for loop
above which starts at the element one *past* the end of Dec, is the
reason you were getting strange output.

> 		printf("%d",Dec[i]);

  putc('0' + Dec[i]);

would also work (I'm not saying it's better, though).

> 		}
> 	
> 	    printf("\n");

  putchar('\n');

Is probably a little more efficient.

> 	}
>     }
> }
> 
> bestimme_laenge(int zaal){
>     //  int zaal;
>     int laenge = 0;
>     while (zaal > 0){
> 	zaal = (zaal - (zaal % 2)) / 2;

Again, this could be shortened to:

  zaal /= 2;

Also, remember to consider the case for zaal == 0 at call time.

> 	laenge = laenge + 1;

Or ++laenge.

<snip>

HTH. If you have further questions, I recommend you post this
dsicussion to comp.lang.c (but read the FAQ, etc. first) as well.

Micah