C Struggling with packing unsigned ints into uint64_t

Question

I am given a collection of data and I am required to pack it into a uint64_t value, which in the following example takes the form of type "weatherlog_t"

I am not allowed to use arithmetic operators (+, ++, -, --, *, %, /, ...), I am allowed however to use bitwise operators (&, |, ^, <<, >>, ~) and logical operators (!, =, --, !=, &&, and ||)

I do however have predefined add() and sub() functions that handle bitwise addition and subtraction, and they are used in the following example. These have been tested and I'm fairly sure they work to the degree needed here.

According to the instructions, the 64-bit value must be arranged as follows:

    /* - year :: 6 bits -- stored as the number of years since the year 2000.
    - month :: 4 bits
    - day :: 5 bits
    - zip_code :: 16 bits
    - high_temp :: in degrees Fahrenheit, stored as an 8-bit signed integer
    - low_temp :: in degrees Fahrenheit, stored as 8-bit signed integer
    - precipitation :: in mm. stored as a 10-bit unsigned integer.
    - average_wind_speed :: 7 bits. unsigned int km/hr.

    All of these are packed into a 64 bit unsigned integer in the above order.

    We'd store:
- year :: 2015, which is 15 years from 2000, so 001111
- month :: September, which is the 9th month, so 1001.
- day :: 16, which is 1 0000
- zip_code :: 19122 which is 0100 1010 1011 0010
- high_temp :: 85F, so 0101 0101
- low_temp :: 65F, so 0100 0001
- precipitation :: 35 mm so 00 0010 0011
- average wind speed :: 5 km/h, so 000 0101

And all would be packed into a single 64-bit unsigned integer:

00 1111 1001 10000 0100 1010 1011 0010 0101 0101 0100 0001 00 0010 0011 000 0101

OR

0011 1110 0110 0000 1001 0101 0110 0100 1010 1010 1000 0010 0001 0001 1000 0101 */

So far, what I have is:

weatherlog_t pack_log_entry(unsigned int year, unsigned int month, unsigned int day,
                        unsigned int zip, int high_temp, int low_temp,
                        unsigned int precip, unsigned int avg_wind_speed) {


weatherlog_t ret = 0;

unsigned int newYear = sub(year, 2000);

ret = (ret << 6);
ret = add(ret, newYear);

ret = (ret << 4);
ret = add(ret, month);

ret = (ret << 5);
ret = add(ret, day);

ret = (ret << 16);
ret = add(ret, zip);

ret = (ret << 8);
ret = add(ret, high_temp);

ret = (ret << 8);
ret = add(ret, low_temp);

ret = (ret << 10);
ret = add(ret, precip);

ret = (ret << 6);
ret = add(ret, avg_wind_speed);


return ret;
}

However, when I go in and test this, checking the binary value of ret, it seems to stop at 32-bits, and shifting left after this point causes any bits left of the 32nd leftmost bit to be lost. I am struggling to understand what I am doing wrong, although I am new at bitwise arithmetic and do not fully understand as of yet how it interacts with the C language.

EDIT: As requested, code for add() and subtract()

unsigned int add(unsigned int i, unsigned int j) {
/* can be done in a total of 7 lines, including one to declare an unsigned int, */
/* two for a while loop, and one for the return
 You're not required to do it in 7 lines though . */
while(j != 0){
    unsigned int carry = i & j;

    i = i ^ j;

    j = carry << 1;
}

return i;
}


unsigned int sub(unsigned int i, unsigned int j) {
/* Similar 7 lines, although there is a shorter way */
while (j != 0){
    unsigned int borrow = (~i) & j;

    i = i ^ j;

    j = borrow << 1;
}

return i;
}

Answer 1

I have no idea what you need those add/sub functions for; seems like obfuscation. Packing data in specific bits is much more straight-forward:

#define YEAR_POS  58
#define MONTH_POS 48

ret = (uint64_t)year  << YEAR_POS  |
      (uint64_t)month << MONTH_POS |
       ...

This have benefits of being 1) fast, and 2) endian-independent = fully portable.

You might have to mask each variable in advance if you suspect that they contain garbage beyond the specified sizes:

#define YEAR_SIZE 6
year &= (1u << YEAR_SIZE)-1; 

Answer 2

I cannot comment due to my lack of reputation.

When you specifically need an integer value to have a certain signedness and width, you can use the types defined in stdint.h. From what I can tell that appears to be one of the issues, given add and substract return an unsigned integer and have it in their arguments - how wide they are is platform dependant. stdint.h guarantees signedness and width. Since you're using these two functions and add the result to a uint64_t, you may lose bytes in the progress.

https://www.gnu.org/software/libc/manual/html_node/Integers.html

If you cannot adjust the return value of add and sub, I would suggest making new ones specifically for this purpose.

Answer 3

Your add and sub functions each take two argument of type unsigned int and return an unsigned int. This type is most likely smaller than 64 bits, so passing a uint64_t to one of these functions truncates the value.

Change the parameter types to weatherlog_t as well as the locals used within the functions and the return types.

weatherlog_t add(weatherlog_t i, weatherlog_t j) {
    /* can be done in a total of 7 lines, including one to declare an unsigned int, */
    /* two for a while loop, and one for the return
     You're not required to do it in 7 lines though . */
    while(j != 0){
        weatherlog_t carry = i & j;

        i = i ^ j;

        j = carry << 1;
    }

    return i;
}

weatherlog_t sub(weatherlog_t i, weatherlog_t j) {
    /* Similar 7 lines, although there is a shorter way */
    while (j != 0){
        weatherlog_t borrow = (~i) & j;

        i = i ^ j;

        j = borrow << 1;
    }

    return i;
}

Answer 4

Problems:

Code is failing to mask upper bits of signed values like high_temp.

Last shift is curiously 6 and not 7.

Pedantic, code fails to insure augments are in range. Another reason to mask to limit out-of-range value form affecting other fields.

"it seems to stop at 32-bits" due to add() limited to 32-bits @dbush. add() not needed anyways.

---

Simply shift , mask, or.

#define N_YEAR 6
#define N_MONTH 4
#define N_DAY 5
#define N_ZIP 16
#define N_HTEMP 8
#define N_LTEMP 8
#define N_PREC 10
#define N_AWS 7
#define MSK(bw) ((1u << (bw)) - 1)

weatherlog_t pack_log_entry(unsigned int year, unsigned int month,
    unsigned int day, unsigned int zip, int high_temp, int low_temp,
    unsigned int precip, unsigned int avg_wind_speed) {

  weatherlog_t ret = 0;
  ret = (ret << N_YEAR)  | (sub(year, 2000) & MSK(N_YEAR));
  ret = (ret << N_MONTH) | (month           & MSK(N_MONTH));
  ret = (ret << N_DAY)   | (day             & MSK(N_DAY));
  //... others
  ret = (ret << N_PREC)  | (precip          & MSK(N_PREC)) ;
  ret = (ret << N_AWS)   | (avg_wind_speed  & MSK(N_AWS));
  return ret;
}