Small Issue with C program to convert lowercase to uppercase for stdout

Question

I am learning about system calls in linux and I wrote a quick program to copy stdin to stdout.

I want to convert the stdin lowercase letters to uppercase letters before they are written.

I implemented a function with a pointer to the array to capitalize the characters. It only capitalizes the first letter of the write though and I don't understand why. From what I understand, I wouldn't need a for loop because of the read() system call.

#include <unistd.h>
#define SIZE 512
char charToUpper(char *);

int main () {
    int nread;
    char buf[SIZE];
    while (nread = read(0, buf , SIZE)) {
        charToUpper(buf);
        write(1,buf,nread);
    }
    return 0;
}

char charToUpper(char *a){
    if ((*a > 96) && (*a <123)) {
        *a = *a-32;
        return *a;
    }
}

Answer 1

Well, you have overcomplicated things, by buffering the input data yourself, while you could use getchar() and putchar() already buffering functions. Also using numbers, instead of the character literals, hides the actual objective of your code (converting to uppercase, against adding subtracting strange magic values to your code), and ties it also to ASCII charcode environments:

#include <stdio.h>

int main()
{
    int c;
    while((c = getchar()) != EOF) {
        if (c >= 'a' && c <= 'z')  /* char literals are your friends */
            c += 'A' - 'a';        /* also here                      */
        putchar(c);
    }
    return 0;
}

This snippet of code will do exactly what you try, transparently buffering the input and output, and in a way portable to other environments with different char encodings. No doubt a better way is to do:

#include <stdio.h>
#include <ctype.h>

int main()
{
    int c;
    while((c = getchar()) != EOF) {
        putchar(toupper(c));
    }
    return 0;
}

If you test these two programs with a large text file, you'll see that this program response is better thant the one you wrote, because the standard library has selected automatically for you the best buffer size for reading and writing, based on the file descriptors passed to it, instead of the fixed buffer size (512 bytes) you use in your program.

In your code, you pass the buffer pointer to your function and it only uppercases the first character of it... nothing more, nothing less, so you'll get only the first char of each line (in case your input is from the terminal, as the input ends on each line while reading from a terminal in canonical mode) or the first character of each 512 byte block, in case you read from a file.

For your function to work, you should pass the buffer, and it's size, as the actual amount of read data is not known inside it, and you need to put a for loop inside it to process each character... or make the loop outside the function, and call the function for each character position of the buffer, filled by read(2).

In case you want to do buffering yourself, as you show in the example code, you have to put the actual lenght of read characters and convert the amount passed in the routine, as in:

#include <unistd.h>
#include <stdio.h> /* for printing errors to stderr */
#include <string.h> /* for strerror */
#include <errno.h> /* for errno definition */

#define SIZE 512

void charToUpper(char *, size_t);

int main () {
    ssize_t nread, nwritten;
    char buf[SIZE];
    while ((nread = read(0, buf , SIZE)) > 0) {
        charToUpper(buf, nread);
        nwritten = write(1, buf, nread);
        if (nwritten < 0) {  /* check for writing errors */
            fprintf(stderr,
                "Error: write: %s\n",
                strerror(errno));
            return 1;  /* write error */ 
        }
    }
    if (nread < 0) {  /* check for reading errors */
        fprintf(stderr,
            "Error: read: %s\n",
            strerror(errno));
        return 2; /* read error */
    }
    return 0; /* no error */
}

void charToUpper(char *a, size_t sz){  /* better declare it void, as we don't return anything */

    /* this code assumes uppercase chars and lower case are together in the
     * char map, and contiguous (this is false in EBCDIC mappings) so better
     * to use toupper(*a) in all cases */
    for(;sz--; a++) {  /* for every char in a, up to sz chars. */
        if ((*a >= 'a') && (*a <= 'z')) {  /* if *a is lower case letter */
            *a += 'A' - 'a'; /* convert to uppercase */
            /* better if you use *a = toupper(*a); */
        }
    }
}

Answer 2

The following proposed code:

1. cleanly compiles
2. properly terminates the input array of characters
3. properly changes all lower case characters to upper case, using the facility: toupper() from the header file: ctype.h
4. performs the desired functionality

and now, the proposed code:

#include <unistd.h>
#include <ctype.h>   // toupper()

#define SIZE 512

void charToUpper(char *);

int main ( void ) 
{
    ssize_t nread;
    char buf[SIZE];
    while ( (nread = read(0, buf , SIZE) ) > 0) 
    {
        // NUL terminate the character string
        buf[nread] = '\0';

        // convert all characters to uppercase
        charToUpper(buf);

        // output the (all uppercase) string to stdout
        write(1,buf, (size_t)nread);
    }
    return 0;
}


void charToUpper( char buf[] )
{
    for( size_t i = 0; buf[i]; i++ )
    {
        buf[i] = (char)toupper( buf[i] );
    }

}

Answer 3

he program is perfectly fine except for the loop part.

char* charToUpper(char *a){
    char *t=p;
    while(*a!='\0'){
    if ((*a > 96) && (*a <123)) 
        *a = *a-32;
    a++;
    }
    return t;
}

you didn't incremented the loop. do it and u will get

Answer 4

Your charToUpper receives a pointer to a char, and you sent it buf, which decays to a pointer to a char of the first character in buf, hence your result.

Remember in c you do not get the size of the array you are passing for free - you have to pass it on as well. Consider all your operations in charToUpper are on *a, which is of type char, a single character. So to fix this change the declaration to

char charToUpper(char *, unsigned int size);

so know you know how many characters you actually read and need to change. Try and change your program now to suit this. One hint - your return will probably have to move for example.