Converting char to unsigned short in C

Question

I've been having some trouble saving char values into a structure field which is of the type unsigned short.

---EDIT: Here's the input .pgm ---

P2
6 10

255

255 255 255 255 10 255

255 255 255 255 20 255

255 255 255 255 30 255

255 255 255 255 40 255

255 255 255 255 50 255

255 255 255 255 60 255

110 255 255 255 70 255

255 100 90 80 255 255

255 255 255 255 255 255

255 255 255 255 255 255

So we're suppose to save a matrix composed of 0's and 1's, or other numbers. The structures are as follows:

typedef struct{
    unsigned short rgb[3];   
}PIXEL_T;

typedef struct{
    int format;
    int nrows;
    int ncolumns;
    int max_color;
    PIXEL_T **pixels;   
}PBM_T;

And here's the code to save the 1 or 0 chars into the appropriate structure field.

void pbmMatrixASCII(char* line, size_t len, FILE* file_stream, PBM_T *img_struct) {

    int i = 0, j = 0;
        char *token;

    while(getline(&line, &len, file_stream) != EOF) {
        token = strtok(line, " \n\t");
        while (token != NULL) {
            //DEBUG("%s", token);
            img_struct->pixels[i][j].rgb[0] = strtol(token, NULL, 0);
            token = strtok(NULL, " \n\t");
            printf("%hu ", img_struct->pixels[i][j].rgb[0]);
            j++;
        }
        printf("\n");   
        i++;
    }
}

Now this works great for 0's and 1's. Problem is, when we have to save a matrix which has values ranging from 1 to 255. It saves and prints garbage. And we tried several casting methods. For example:

img_struct->pixels[i][j].rgb[0] = (unsigned short) token;

They don't work. How can we fix this? (Also, is there a PROPER way to actually convert the values, one that serves for both problems?)

----EDIT in response to some feedback----

We're reading from pbm and pgm files. The second line of which is the size of the matrix, which we then read to the img_struct->nrows and img_struct->ncolumns. Here's the code:

//Detects the filetype and reads the matrix size (also reads max color in case of pgm)

//Reads the first line to determine the magic number    

    getline(&line, &len, file_stream);
    sscanf(line, "P%d", &aux_format);

    //Verifies if the format is valid. Exits if not

    if(aux_format > 6 || aux_format < 1) {
        ERROR(ERR_FORMAT,"Invalid format\n");
    }

    img_struct->format = aux_format; //Saves the format into the structure, after being verified

    int size_is_read = 0; //Variable used to verify if the size has been read (a makeshift boolean)

        while(getline(&line, &len, file_stream) != EOF){

            if (hasCommentsOrEmptyLines(line))
                continue;

    //Reads the size of the matrix

            if(size_is_read == 0) {
                sscanf(line,"%d %d",&columns,&rows);
                size_is_read = 1;
            }

            if(img_struct->format == 1 || img_struct->format == 4) { //P1 and P4 are .pbm files and therefore don't have  a maximum colour field
                break;

            } else if(img_struct->format == 2 || img_struct->format == 5){ //Every other file type needs to have its maximum colour field read

                while(getline(&line, &len, file_stream) != EOF) {

                    if(hasCommentsOrEmptyLines(line))
                        continue;

    //reads the max color                   
                    sscanf(line,"%d",&(img_struct->max_color));

                break;
            }

        }
        break;
    }

And after that, the memory alloc followed by the matrix reader function call (see above):

//Save the image size in the appropriate structure fields   
img_struct->nrows = rows;
img_struct->ncolumns = columns;

//Allocates the appropriate size of the matrix to save the bitmap

img_struct->pixels = MALLOC(sizeof(PIXEL_T *)*rows);
if (img_struct->pixels == NULL)
    ERROR(ERR_ALLOC,"Error allocating memory for the bitmap matrix");

for(i = 0; i < rows; i++) {
    img_struct->pixels[i] = MALLOC(sizeof(PIXEL_T)*columns);
}


//Iterate each position of the line array and convert it to an unsigned short 

switch(img_struct->format) {

    case 1:
        pbmMatrixASCII(line, len, file_stream, img_struct);
    break;

    case 2:
        printf("teste\n");
        pgmMatrixASCII(line, len, file_stream, img_struct);
    break;

Answer 1

Instead of using C functions for tokenizing the string, try simply iterating through it like so:

void pbmMatrixASCII(char* line, size_t len, FILE* file_stream, PBM_T *img_struct) {
    int i = 0;

    while(getline(&line, &len, file_stream) != EOF) {
        // Extract all pixel entries.
        int nFound = getPixels(line, img_struct->ncolumns, img_struct->pixels[i]);

        // Print out what we found.
        // int j = 0;
        // for (j = 0; j < nFound; j++) {
        //     printf("%hu %s",
        //         img_struct->pixels[i][j].rgb[0],
        //         ((j + 1) == nFound ? "\n": ""));
        // }

        // Go to the next row of pixels if we found values.
        if (nFound == img_struct->ncolumns)
            i++;
    }
}

And have another function parse each line you read in:

int getPixels(char * line, int numExpected, PIXEL_T * pixelRow) {
    int max                = strlen(line);
    int counter            = 0;
    PIXEL_T * currentPixel = pixelRow;
    char * linePtr         = line;

    while (counter < numExpected) {
        // Reach the first non-whitespace character.
        while (linePtr[0] == ' ') {
            if ((linePtr - line) >= max)
                return (counter);

            linePtr++;
        }

        if (linePtr[0] == '\n')
            return (counter);

        // Grab the unsigned short value.
        int numFound = sscanf(linePtr, "%hu", &currentPixel->rgb[0]);

        // Make sure we found something.
        if (numFound > 0)
            currentPixel++, counter++;
        // Error happened with sscanf! Return what we found so far.
        else if (numFound < 0)
            return (counter);

        // Reach the first non-NON-whitespace character. Double negative ;)
        while (linePtr[0] != ' ') {
            if ((linePtr - line) >= max)
                return (counter);

            linePtr++;
        }
    }
    return (counter);
}

This way, you never have to actually cast the value from a character; it's read as an unsigned short by sscanf(...).

-- EDIT: The following is for reading from PPM files --

Note: numExpected is the number of expected RGB triplets you'll see. Passing in a value of 2 means there should be 2 pixels there, each with 3 values, totaling to 6 actual entries on the line read in. This corresponds to the width value in the top part of the PPM file, itself.

int getPixels(char * line, int numExpected, PIXEL_T * pixelRow) {
    int max                = strlen(line);
    int counter            = 0;
    int currentValue       = 0;
    PIXEL_T * currentPixel = pixelRow;
    char * linePtr         = line;

    while (counter < numExpected) {
        while (currentValue < 3) {
            // Reach the first non-whitespace character.
            while (linePtr[0] == ' ') {
                if ((linePtr - line) >= max)
                    return (counter);

                linePtr++;
            }

            if (linePtr[0] == '\n') {
                return (counter);
            }

            // Grab the unsigned short value.
            int numFound = sscanf(linePtr, "%hu", &currentPixel->rgb[currentValue]);

            // Make sure we found something.
            if (numFound == 1)
                currentValue++;

            // Error happened with sscanf! Return what we found so far.
            else if (numFound < 0)
                return (counter);

            // Reach the first non-NON-whitespace character. Double negative ;)
            while (linePtr[0] != ' ') {
                if ((linePtr - line) >= max)
                    return (currentValue == 3 ? counter + 1 : counter);

                linePtr++;
            }
        }

        counter++, currentPixel++, currentValue = 0;
    }
    return (counter);
}

Answer 2

So in your file format you have : row = 6 column = 10

The content is :

R G B R G B

R G B R G B

.

.

.

The row value is not the number of pixels but the number of pixels * 3. So 2 pixels per row.

You store your pixel data as : Pixel_t **data; -> pointer of a pointer of a structure containing a static array of 3 unsigned shorts.

You allocate the data that way :

data = malloc(sizeof(Pixel_t*) * row);

so now data is an array of pointers of pixel_t of size 'row'

then you allocate each pointer of pixel_t : data[n] = malloc(sizeof(pixel_t) * column); so now for each row you basically have an array of size of 3 unsigned short * columns which makes it 3 times too big because you allocate a pixel for each column.

When you loop through your data you always write to :

data[y][x].rgb[0]

which is exactly like :

*(data[y][x])

which makes your structure useless because every-time you access to the first element of your array... So depending after how you output your data, you will have garbage at the end of you array because you never actually use it since it's 3 times too big.

Also using strtok and strtol and store it to a unsigned short is supposed to work fine in your case so the parsing is not the problem.

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

int main()
{
    char *str = strdup("255 42 25 255 67 89\n");
    char *token = strtok(str, " \n");
    while (token)
    {
        unsigned short val = strtol(token, 0, 0);
        token = strtok(0, " \n");
        printf("%hu\n", val);
    }

    return 0;
}

This code outputs the correct data.

Answer 3

Sorry, i cant post a comment, just an answer. Have you ever tried to use a debugger?? Its made for this purpose. You can step through every line of code and watch the current data of your variables. See whats happening when you read 0/1 and again when you read other values. Then you can exactly locate the error.

mfg

Just modified your read-function a bit and tried it on my on:

static const char filename[] = "ok.pgm";
FILE *file = fopen ( filename, "r" );

while(fgets(line, sizeof line, file) != NULL) {
    token = strtok(line, " \n\t");

    while(token != NULL) {
        //DEBUG("%s", token);
        long int t = strtol(token, NULL, 0);
        token = strtok(NULL, " \n\t");
        printf("%hu ", t);
        j++;
    }
    printf("\n");
    i++;
}

Output:

0
6 10
255
255 255 255 255 10 255
255 255 255 255 20 255
255 255 255 255 30 255
255 255 255 255 40 255
255 255 255 255 50 255
255 255 255 255 60 255
110 255 255 255 70 255
255 100 90 80 255 255
255 255 255 255 255 255
255 255 255 255 255 255

Process returned 0 (0x0)   execution time : 0.016 s
Press any key to continue.

Writing it to an unsigned short provides the same output. It seems you have a problem with your struct...

BTW: Code is bad, just an example if the reading works.

Answer 4

In the code line:

img_struct->pixels[i][j].rgb[0] = (unsigned short) token;

You are assigning token which is a pointer to unsigned short. When you cast it you transform pointer to unsigned int. That may looks like a garbage. You can fix it in the following way:

unsigned int value = (unsigned int)atoi(token);
img_struct->pixels[i][j].rgb[0] = value;

In the snippet below loop counter j is always incremented. Thus when reading second large line and accessing pixel array crash happens.

int i = 0, j = 0;
    char *token;

while(getline(&line, &len, file_stream) != EOF) {
    token = strtok(line, " \n\t");
    j = 0;                                          // <-- FIXED
    while (token != NULL) {
        //DEBUG("%s", token);
        img_struct->pixels[i][j].rgb[0] = strtol(token, NULL, 0);
        token = strtok(NULL, " \n\t");
        printf("%hu ", img_struct->pixels[i][j].rgb[0]);
        j++;
    }
    printf("\n");   
    i++;
}