On Sun, Feb 18, 2024 at 20:09 AM eric sunshine <sunshine@xxxxxxxxxxxxxx> wrote: > The code in question is not being used as a "bag of bits". Rather, > it's a tristate binary with values "not-set", "true", and "false". > Whereas a typical binary could be represented by a single bit, this > one needs the extra bit to handle the "not-set" case. Moreover, it is > idiomatic in the Git codebase for -1 to represent "not-set", so I > think this code is fine as-is since its meaning is clear to those > familiar with the codebase, thus does not need any changes made to it. Thank you for the clarification and sorry for the misunderstanding. > So, refresh_index() is correctly expecting an unsigned value for > `flags` but refresh() in `builtin/add.c` has undesirably declared > `flags` as signed. So, an unsigned type is preferable since we are dealing with 'bags of bits', and probably only bitwise operators operate on them. Also the mixing is not ideal. Yes, I'm interested in fixing the one in `builtin/add.c`. Il giorno mar 20 feb 2024 alle ore 00:39 eugenio gigante <giganteeugenio2@xxxxxxxxx> ha scritto: > > On Sun, Feb 18, 2024 at 20:09 AM eric sunshine > <sunshine@xxxxxxxxxxxxxx> wrote: > > The code in question is not being used as a "bag of bits". Rather, > > it's a tristate binary with values "not-set", "true", and "false". > > Whereas a typical binary could be represented by a single bit, this > > one needs the extra bit to handle the "not-set" case. Moreover, it is > > idiomatic in the Git codebase for -1 to represent "not-set", so I > > think this code is fine as-is since its meaning is clear to those > > familiar with the codebase, thus does not need any changes made to it. > > Thank you for the clarification and sorry for the misunderstanding. > > > So, refresh_index() is correctly expecting an unsigned value for > > `flags` but refresh() in `builtin/add.c` has undesirably declared > > `flags` as signed. > > So, an unsigned type is preferable since we are dealing > with 'bags of bits', and probably only bitwise operators operate > on them. Also the mixing is not ideal. > Yes, I'm interested in fixing the one in `builtin/add.c`.
