C program from hardware engineer with very little coding background

Question

I am trying to debug this program which I have written, but all I get is the garbage values.

/*
 *  Assignment.c
 *
 *  Created on: 18-Aug-2017
 *  Author: sumeetpatil
 */

#include <stdio.h>
#include <string.h>
#include <stdlib.h>
#include <stdio.h>

char rotateArray(char str[], int degree, int length) {
    char temp[length];
    for (int i = 0; i < length + 1; i++) {
        str[i] = str[i] +  &degree;
        str[i] = temp[i];
        //printf ("%c", str[i]);
    }
    return &str;
}

int main() {
    int length;
    int degree;
    char encryptArr[50];

    printf("Enter the value of degree: ", "\n");

    scanf("%d", &degree, "\n");

    printf("Enter the string array you want to encrypt: ", "\n");

    fgets(encryptArr, 100, stdin);

    scanf("%[^\n]%*c", &encryptArr);

    length = strlen(encryptArr);

    printf("The character string is: %s\n", encryptArr, "\n");

    printf("The size of character array is %u\n", length);

    rotateArray(encryptArr, degree, length);

    printf("%s\n", rotateArray);
}

My output is as follows:

Enter the value of degree: 1
Enter the string array you want to encrypt: youmadbro
The character string is: youmadbro
The size of character array is 9
UH��H��@H�a

Please let me know what is going wrong in this program.

Answer 1

Use this:

char *rotateArray(char *str, int degree, int length) {
    for (int i = 0; i < length; i++) {
        char c = str[i];
        if (c >= 'a' && c <= 'z') {
            str[i] = 'a' + (c - 'a' + degree) % 26;
        } else
        if (c >= 'A' && c <= 'Z') {
            str[i] = 'A' + (c - 'A' + degree) % 26;
        }
    }
    return str;
}

Answer 2

The encoding function has problems:

• the loop index should vary from 0 to len - 1 inclusive, hence the test should be:

  for (int i = 0; i < length; i++) { ...
  

• shifting the character values by degree must be done more carefully, handling lowercase and uppercase characters separately and using a modulo computation for a circular permutation.

• to use the value of degree, you do not need the & addressof operator.

• the string is modified in place, you do not need a temporary array.

Here is a corrected version:

char *rotateArray(char *str, int degree, int length) {
    for (int i = 0; i < length; i++) {
        char c = str[i];
        if (c >= 'a' && c <= 'z') {
            str[i] = 'a' + (c - 'a' + degree) % 26;
        } else
        if (c >= 'A' && c <= 'Z') {
            str[i] = 'A' + (c - 'A' + degree) % 26;
        }
    }
    return str;
}

The main() function has problems too:

• fgets() and scanf() are 2 different ways to read the string, just use one.

• The second argument to fgets(encryptArr, 100, stdin); should be 50 or sizeof(encryptArr) to tell fgets() the size of the destination array and prevent buffer overflow.

• You pass extra "\n" arguments to scanf() and printf() that are ignored. Enable compiler warnings (-Wall -Wextra) and study the messages.

• function rotateArray() modifies encryptArr in place, you should pass encryptArr to printf() instead of the function name.

Here is a corrected version:

int main(void) {
    int length;
    int degree;
    char encryptArr[50];

    printf("Enter the value of degree: ");
    if (scanf("%d", &degree) != 1)
        return 1;

    printf("Enter the string array you want to encrypt: ");
    if (!fgets(encryptArr, sizeof(encryptArr), stdin))
        return 1;

    length = strlen(encryptArr);
    printf("The character string is: %s\n", encryptArr);
    printf("The length of character string is %d\n", length);

    rotateArray(encryptArr, degree, length);
    printf("%s\n", encryptArr);

    return 0;
}

Finally, you include <stdio.h> twice. While it does not cause a problem, you should only include header files once and only those needed:

#include <stdio.h>
#include <string.h>

Answer 3

Your code contains some behaviours that are wrong.

I guess maybe you're coming from another language and attempting to learn C?

Well, let us see what kind of errors in the code that you've made.

---

1.

char rotateArray(char str[], int degree, int length) {

    char temp[length];
    for(int i=0; i < length +1; i++)
    {
        str[i] = str[i]+ &degree;
        str[i] = temp[i];
        //printf ("%c", str[i]);
        }
    return &str;
}

My guess is you're trying to rotate (shift the character) one by one and storing it into the temp string array. You don't actually need to store the new rotated value into the a new temp because str array has already been modified here str[i] = str[i]+ &degree. The reason is that in C, an array is passed kind of like reference so you can modify the value and it still gonna 'affects' the str's value outside of it.

So we can remove temp array altogether.

/* char rotateArray(char str[], int degree, int length) { */
void rotateArray(char str[], int degree, int length) {

    /* char temp[length]; */

    for(int i=0; i < length +1; i++)
    {
        str[i] = str[i]+ degree;
        /* str[i] = temp[i]; */
        //printf ("%c", str[i]);
    }
    /* return &str; */
}

---

2.

In the main body, almost on every printf and scanf you've inserted \n at the back. I guess again maybe this style of coding was influenced by another programming language? :)

Actually, in C, you don't have to put the \n as a new parameter unless you had put specifier at the format string. So you can remove , "\n" altogether.

/* printf("Enter the value of degree: ", "\n"); */
printf("Enter the value of degree: ");

/* scanf("%d",&degree, "\n"); */
scanf("%d",&degree);

/* printf("Enter the string array you want to encrypt: ", "\n"); */
printf("Enter the string array you want to encrypt: ");

/* printf("The character string is: %s\n", encryptArr, "\n"); */
printf("The character string is: %s\n", encryptArr);

printf & scanf is kinda like a beast to a new C programmer. Please take a look at more resources that discuss those matter.

---

3.

Lastly, at the end of your main body,

printf("%s\n", rotateArray);

This is kinda nonsense because you're trying to print the function name. Well, I guess again maybe you thought that it will print the returned value of the function, right?

Actually, from what I have told you earlier at the top, any string array that passed into the function as a str parameter is kinda like a reference, so when you're modifying str inside the rotateArray body it will 'affect' the value of the main function too.

So instead of that wrong code, you can change it to be like this,

/* printf("%s\n", rotateArray); */
printf("%s\n", encryptArr);

---

So, the final modified code will be:

/*
 *  Assignment.c
 *
 *  Created on: 18-Aug-2017
 *  Author: sumeetpatil
 */

#include <stdio.h>
#include <string.h>
#include <stdlib.h>
#include <stdio.h>

/* char rotateArray(char str[], int degree, int length) { */
void rotateArray(char str[], int degree, int length) {

    /* char temp[length]; */

    for(int i=0; i < length +1; i++)
    {
        str[i] = str[i]+ degree;
        /* str[i] = temp[i]; */
        //printf ("%c", str[i]);
    }
    /* return &str; */
}

int main(){

    int length;
    int degree;
    char encryptArr[50];

    /* printf("Enter the value of degree: ", "\n"); */
    printf("Enter the value of degree: ");
    fflush(stdout);

    /* scanf("%d",&degree, "\n"); */
    scanf("%d",&degree);

    /* printf("Enter the string array you want to encrypt: ", "\n"); */
    printf("Enter the string array you want to encrypt: ");
    fflush(stdout);

    fgets(encryptArr, 100, stdin);

    scanf ("%[^\n]%*c", &encryptArr);

    length = strlen(encryptArr);

    /* printf("The character string is: %s\n", encryptArr, "\n"); */
    printf("The character string is: %s\n", encryptArr);

    printf("The size of character array is %u\n", length);

    rotateArray(encryptArr, degree, length);

    /* printf("%s\n", rotateArray); */
    printf("%s\n", encryptArr);

}

---

Bonus

What're you trying to achieve here is what called as Caesar cipher.

Your implementation was wrong because it doesn't handle 'alphabet overflow' correctly. What I mean is when your code exceeds z it doesn't rotate itself to a again. There are many ways to achieve this either by using remainder operator % or using if-else statement to check either the alphabet has overflowed.

Try to look at the example here.

Answer 4

Let's start by what is not ok.

scanf ("%[^\n]%*c", &encryptArr); //Wrong
scanf ("%[^\n]%*c", &encryptArr[0]); //OK
scanf ("%[^\n]%*c", encryptArr); //OK

Now your rotation function.

char rotateArray(char str[], int degree, int length) {
    char temp[length]; //Declared on stack, values are not known, UB
    for(int i=0; i < length + 1; i++) { //Why +1? Forces UB in later stage
        str[i] = str[i]+ &degree; //Degree is variable on stack, adding & is just getting address from stack
        str[i] = temp[i]; //Assigning temp value with undefined value
        //printf ("%c", str[i]); //Printing undefined value
        }
    return &str; //You should get at least warning here.
    return str[0]; //Better
}

This is now generic. When you will tell us what is desired output according to input, I will be able to tell you more.

printf("%s\n", rotateArray);

You cannot just print function name as string. This is undefined behavior in C.