procfs icon indicating copy to clipboard operation
procfs copied to clipboard

Change stat data types to match the related kernel data types

Open vykulakov opened this issue 4 years ago • 2 comments

The man page for /proc/[pid]/stat describes data types that should be used to parse particular values in this stat file. So the following information about data types may be obtained from there:

(1) pid  %d
(2) comm  %s
(3) state  %c
(4) ppid  %d
(5) pgrp  %d
(6) session  %d
(7) tty_nr  %d
(8) tpgid  %d
(9) flags  %u
(10) minflt  %lu
(11) cminflt  %lu
(12) majflt  %lu
(13) cmajflt  %lu
(14) utime  %lu
(15) stime  %lu
(16) cutime  %ld
(17) cstime  %ld
(18) priority  %ld
(19) nice  %ld
(20) num_threads  %ld
(21) itrealvalue  %ld
(22) starttime  %llu
(23) vsize  %lu
(24) rss  %ld
(25) rsslim  %lu
(26) startcode  %lu  [PT]
(27) endcode  %lu  [PT]
(28) startstack  %lu  [PT]
(29) kstkesp  %lu  [PT]
(30) kstkeip  %lu  [PT]
(31) signal  %lu
(32) blocked  %lu
(33) sigignore  %lu
(34) sigcatch  %lu
(35) wchan  %lu  [PT]
(36) nswap  %lu
(37) cnswap  %lu
(38) exit_signal  %d  (since Linux 2.1.22)
(39) processor  %d  (since Linux 2.2.8)
(40) rt_priority  %u  (since Linux 2.5.19)
(41) policy  %u  (since Linux 2.5.19)
(42) delayacct_blkio_ticks  %llu  (since Linux 2.6.18)
(43) guest_time  %lu  (since Linux 2.6.24)
(44) cguest_time  %ld  (since Linux 2.6.24)
(45) start_data  %lu  (since Linux 3.3)  [PT]
(46) end_data  %lu  (since Linux 3.3)  [PT]
(47) start_brk  %lu  (since Linux 3.3)  [PT]
(48) arg_start  %lu  (since Linux 3.5)  [PT]
(49) arg_end  %lu  (since Linux 3.5)  [PT]
(50) env_start  %lu  (since Linux 3.5)  [PT]
(51) env_end  %lu  (since Linux 3.5)  [PT]
(52) exit_code  %d  (since Linux 3.5)  [PT]

Here things like %d and %lu are scanf(3) format specifiers (according to the man page). The full list of the used above specifiers is:

  • %d - for int;
  • %u - for unsigned int;
  • %ld - for long;
  • %lu - for unsigned long;
  • %llu - for unsigned long long;
  • %s - matches a sequence of non-white-space characters;
  • %c - matches a sequence of characters whose length is specified by the maximum field width (default 1).

But in the code we have the following (just a few examples):

UTime uint
STime uint
CUTime uint
CSTime unit

So instead of parsing UTime into unsigned long (according to the man page), the library is trying to parse it into uint that may be uint32 on machines with the 32-bit arch. As a result, we may get the value out of range errors. With CUTime it is even worse: instead of signed long the library is trying to use uint so any negative values may raise errors too.

Such errors have been already faced in real use. Check #401 for details.

I propose to fix the field types according to the list above. It will break compatibility with any old code but as the library is only on the pre-1.0 stage it can be done easily. Additionally, in golang_client such values are converted into float64 so the future migration may happen unnoticed.

Useful links:

  • https://man7.org/linux/man-pages/man5/proc.5.html
  • https://man7.org/linux/man-pages/man3/scanf.3.html
  • https://github.com/prometheus/procfs/pull/402#issuecomment-887545268

vykulakov avatar Jul 27 '21 14:07 vykulakov

Well, I made a mistake in my conclusion. In C they just specify the minimum size of data types like int and long. So the real data type sizes depend on implementations and finally on particular machine architectures.

You may check the Wikipedia for details and other resources:

  • https://en.wikipedia.org/wiki/C_data_types
  • https://www.ibm.com/docs/en/ibm-mq/9.2?topic=platforms-standard-data-types-aix-linux-windows

As you may guess, they do the same in the Go for the int and uint data types so these data types may take 4 or 8 bytes as well as long and unsigned long do in C.

As a result, the existent code to parse the stat fields work perfectly except for the fields CUTime and CSTime - they both should be signed long or just int in Go instead of uint currently.

vykulakov avatar Jul 29 '21 11:07 vykulakov

I'll prepare a fix for those two fields.

vykulakov avatar Jul 29 '21 11:07 vykulakov

ACK, I believe this can be closed now.

rexagod avatar Mar 22 '24 12:03 rexagod

All seems to be fine so far, closing the issue.

vykulakov avatar Mar 22 '24 13:03 vykulakov