Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

kvprintf(): Fix '+' conversion handling #1310

Merged
merged 26 commits into from
Sep 6, 2024

Conversation

sebhub
Copy link
Contributor

@sebhub sebhub commented Jul 1, 2024

For example, printf("%+i", 1) prints "+1". However, kvprintf() did print just "1" for this example. According to PRINTF(3):

A sign must always be placed before a number produced by a signed conversion.

For "%+r" radix conversions, keep the "+" handling as it is, since this is a non-standard conversion.

This change allows to support the ' ' conversion modifier in the future.

@bsdimp
Copy link
Member

bsdimp commented Jul 3, 2024

This looks like it changes the meaning of both '%-p' and '%+p'. before it would always set sign to 0. Now it doesn't which seems like it would be weird.

@sebhub
Copy link
Contributor Author

sebhub commented Jul 3, 2024

I have to admit that I did test the '%+p' using glibc. It seems that it is not defined by POSIX.

#include <stdio.h>

void print(void *p)
{
   printf("'%20p' '%-20p' '%+20p'\n", p, p, p);
}

int main()
{
  print(0);
  print((void*)1L);
  print((void*)-1L);
  return 0;
}

On FreeBSD:

cc -Wall -Wextra -pedantic main.c
main.c:5:29: warning: flag '+' results in undefined behavior with 'p' conversion specifier [-Wformat]
   printf("'%20p' '%-20p' '%+20p'\n", p, p, p);
                           ~^~~~
1 warning generated.
./a.out
'                 0x0' '0x0                 ' '                 0x0'
'                 0x1' '0x1                 ' '                 0x1'
'  0xffffffffffffffff' '0xffffffffffffffff  ' '  0xffffffffffffffff'

On glibc:

cc -Wall -Wextra -pedantic main.c
main.c: In function ‘print’:
main.c:5:32: warning: '+' flag used with ‘%p’ gnu_printf format [-Wformat=]
    printf("'%20p' '%-20p' '%+20p'\n", p, p, p);
                                ^
./a.out
'               (nil)' '(nil)               ' '               (nil)'
'                 0x1' '0x1                 ' '                +0x1'
'  0xffffffffffffffff' '0xffffffffffffffff  ' ' +0xffffffffffffffff'

@sebhub
Copy link
Contributor Author

sebhub commented Jul 4, 2024

Please let me know if you are in principle fine with this change. I can change the '%+p' behaviour to match with the FreeBSD user space.

@sebhub sebhub force-pushed the kvprintf-fix-plus-conversion branch from dfe8b20 to 89ec2c3 Compare July 12, 2024 05:59
@sebhub
Copy link
Contributor Author

sebhub commented Jul 12, 2024

I changed the patch to keep the current "%+p" behaviour.

@bsdimp
Copy link
Member

bsdimp commented Jul 19, 2024

Sorry missed these comments... I think I'm fine with the change (either way, but happier with it matching user space better). I started looking at this, then got distracted and didn't get back to it.

@sebhub
Copy link
Contributor Author

sebhub commented Aug 9, 2024

Anything left to do on my side?

@bsdimp
Copy link
Member

bsdimp commented Aug 9, 2024

Anything left to do on my side?

No. I think it's ready to go, but am not sure. Was going to look closely on my next pr landing day... but last friday and this have gotten away from me so it will be maybe monday or tiesday this next week when i hope to do a mini one.

@bsdimp bsdimp force-pushed the kvprintf-fix-plus-conversion branch from 89ec2c3 to 6197105 Compare September 6, 2024 18:33
bsdimp pushed a commit to VexedUXR/freebsd-src that referenced this pull request Sep 6, 2024
For example, printf("%+i", 1) prints "+1".  However, kvprintf() did
print just "1" for this example.  According to PRINTF(3):

  A sign must always be placed before a number produced by a signed
  conversion.

For "%+r" radix conversions, keep the "+" handling as it is, since this
is a non-standard conversion.  For "%+p" pointer conversions, continue
to ignore the sign modifier to be in line with libc.

This change allows to support the ' conversion modifier in the future.

Reviewed by: imp
Pull Request: freebsd#1310
sebhub and others added 17 commits September 6, 2024 12:34
For example, printf("%+i", 1) prints "+1".  However, kvprintf() did
print just "1" for this example.  According to PRINTF(3):

  A sign must always be placed before a number produced by a signed
  conversion.

For "%+r" radix conversions, keep the "+" handling as it is, since this
is a non-standard conversion.  For "%+p" pointer conversions, continue
to ignore the sign modifier to be in line with libc.

This change allows to support the ' conversion modifier in the future.

Reviewed by: imp
Pull Request: freebsd#1310
10ms seems to be too strict for some configurations, so increase to
20ms.

Reviewed by: imp
Pull Request: freebsd#1327
Add version information to libxo output so that
libxo content consumers can track changes.

Reviewed by: imp, markj
Pull Request: freebsd#1350
Add version information to libxo output so that
libxo content consumers can track changes.

Reviewed by: imp, markj
Pull Request: freebsd#1350
Add version information to libxo output so that
libxo content consumers can track changes.

Reviewed by: imp, markj
Pull Request: freebsd#1350
Add version information to libxo output so that
libxo content consumers can track changes.

Reviewed by: imp, markj
Pull Request: freebsd#1350
Add version information to libxo output so that
libxo content consumers can track changes.

Reviewed by: imp, markj
Pull Request: freebsd#1350
Add version information to libxo output so that
libxo content consumers can track changes.

Reviewed by: imp, markj
Pull Request: freebsd#1350
Closes:		280538
Fixes:		cf8a18 (back out logging to /var/log/adduser)
MFC after:	3 days
Reported by:	Herbert Baerschneider <[email protected]>

Reviewed by: imp
Pull Request: freebsd#1354
This provides functionality for a click which is partially unreleased
and then allows the user to continue moving the mousepad as if were not
invoked as a full click

Signed-off-by: Joshua Rogers <[email protected]>
Reviewed by: imp, wulf
Pull Request: freebsd#1365
This patch allows scrolling with multiple fingers simultaneously, in
line with how wsp trackpads function on MacOS.

Two new tunables are added: hw.usb.wsp.max_finger_area and
hw.usb.wsp.max_double_tap_distance.

max_finger_area defines the maximum size which the driver registered an
object on trackpad as a finger.
Previously, this value was hardcoded as 1200, which was too low to
register thumb-clicks.

max_double_tap_distance defines the maximum distance between two
fingers which will register as a double-click.

Signed-off-by: Joshua Rogers <[email protected]>
Reviewed by: imp, wulf
Pull Request: freebsd#1365
Also correctly use tun.max_double_tap_distance for maximum distance
of fingers for vertical scrolling.

Signed-off-by: Joshua Rogers <[email protected]>
Reviewed by: imp, wulf
Pull Request: freebsd#1365
The struct timespec tv_sec member is of type time_t.  Make sure that all
variables related to this member are of the type time_t.  This is important for
targets where long is a 32-bit type and time_t a 64-bit type.

Reviewed by: imp
Pull Request: freebsd#1373
Commit e695500 updated the policy table
to match RFC 6724, which obsoletes RFC 3484.

Add a reference to RFC 6724, and mark it up as a technical report (%R).

MFC after:	3 days
Signed-off-by:	Jose Luis Duran <[email protected]>

Reviewed by: imp, glebius
Pull Request: freebsd#1375
Update the sample ip6addrctl.conf.sample file to match the default
policy, currently based on RFC 6724.

MFC after:	3 days
Signed-off-by:	Jose Luis Duran <[email protected]>

Reviewed by: imp, glebius
Pull Request: freebsd#1375
Reviewed by: imp, glebius
Pull Request: freebsd#1375
valpackett and others added 9 commits September 6, 2024 12:34
The error was always returned, even after handling the sysctl, breaking
installworld under Linux.

Sponsored by:		https://www.patreon.com/valpackett

Reviewed by: imp
Pull Request: freebsd#1376
MFC after:	3 days

Reviewed by: imp
Pull Request: freebsd#1378
MFC after:	3 days

Reviewed by: imp
Pull Request: freebsd#1379
Fixes:		86c06f (Remove GEOM_SCHED class and gsched)
MFC after:	3 days

Reviewed by: imp
Pull Request: freebsd#1380
MFC after:	3 days

Reviewed by: imp
Pull Request: freebsd#1382
+ consistent document description languague with other USB-BaseT drivers
+ mention newly added adapters from 6ea4d9
+ attempt to mention rgephy(4) phys feed into ure interfaces

Fixes:		6ea4d9 (Move RTL8156 from cdce(4) to ure(4))
MFC after:	3 days

Reviewed by: imp
Pull Request: freebsd#1384
Signed-off-by: Tom Hukins <[email protected]>
Reviewed by: imp
Pull Request: freebsd#1385
Add logic that checks if the code doesn't overflow
ACPI_EXTENDED_HID_DEVICE_PATH node when searching for optional
strings. If the string is not provided in the device path node
default value of "\0" is used.

Upstream PR:	https://bugzilla.tianocore.org/show_bug.cgi?id=4555
Obtained from:	tianocore/edk2@96ed60d

Reviewed by: imp
Pull Request: freebsd#1388
Since 26b9e1f codel was fixed but traffic was not flowing for
pie too. Apply the same fix.

MFC after:	1 week
Sponsored by:	OPNsense
Differential Revision:	https://reviews.freebsd.org/D46182
Also see:	https://redmine.pfsense.org/issues/13996
Also see:	https://forum.opnsense.org/index.php?topic=41827.0
Reviewed by: imp, markj
Pull Request: freebsd#1390
@bsdimp bsdimp force-pushed the kvprintf-fix-plus-conversion branch from 6197105 to 2b7f289 Compare September 6, 2024 18:34
@freebsd-git freebsd-git merged commit 2b7f289 into freebsd:main Sep 6, 2024
102 of 121 checks passed
verm pushed a commit to RTEMS/rtems that referenced this pull request Sep 13, 2024
For example, printf("%+i", 1) prints "+1".  However, kvprintf() did
print just "1" for this example.  According to PRINTF(3):

  A sign must always be placed before a number produced by a signed
  conversion.

For "%+r" radix conversions, keep the "+" handling as it is, since this
is a non-standard conversion.  For "%+p" pointer conversions, continue
to ignore the sign modifier to be in line with libc.

This change allows to support the ' conversion modifier in the future.

Reviewed by: imp
Pull Request: freebsd/freebsd-src#1310
bsdjhb pushed a commit to bsdjhb/cheribsd that referenced this pull request Dec 3, 2024
For example, printf("%+i", 1) prints "+1".  However, kvprintf() did
print just "1" for this example.  According to PRINTF(3):

  A sign must always be placed before a number produced by a signed
  conversion.

For "%+r" radix conversions, keep the "+" handling as it is, since this
is a non-standard conversion.  For "%+p" pointer conversions, continue
to ignore the sign modifier to be in line with libc.

This change allows to support the ' conversion modifier in the future.

Reviewed by: imp
Pull Request: freebsd/freebsd-src#1310
bsdjhb pushed a commit to bsdjhb/cheribsd that referenced this pull request Dec 4, 2024
For example, printf("%+i", 1) prints "+1".  However, kvprintf() did
print just "1" for this example.  According to PRINTF(3):

  A sign must always be placed before a number produced by a signed
  conversion.

For "%+r" radix conversions, keep the "+" handling as it is, since this
is a non-standard conversion.  For "%+p" pointer conversions, continue
to ignore the sign modifier to be in line with libc.

This change allows to support the ' conversion modifier in the future.

Reviewed by: imp
Pull Request: freebsd/freebsd-src#1310
bsdjhb pushed a commit to bsdjhb/cheribsd that referenced this pull request Dec 4, 2024
For example, printf("%+i", 1) prints "+1".  However, kvprintf() did
print just "1" for this example.  According to PRINTF(3):

  A sign must always be placed before a number produced by a signed
  conversion.

For "%+r" radix conversions, keep the "+" handling as it is, since this
is a non-standard conversion.  For "%+p" pointer conversions, continue
to ignore the sign modifier to be in line with libc.

This change allows to support the ' conversion modifier in the future.

Reviewed by: imp
Pull Request: freebsd/freebsd-src#1310
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.