NotificationsYou must be signed in to change notification settings
Fork2.5k
Star10k

util: suppress some uninitialized variable warnings#6659

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Merged

ethomson merged 5 commits intolibgit2:mainfromboretrk:fixwarn

Dec 14, 2023

Merged

util: suppress some uninitialized variable warnings#6659

ethomson merged 5 commits intolibgit2:mainfromboretrk:fixwarn

Dec 14, 2023

Conversation

Copy link

Contributor

boretrk commentedNov 17, 2023

A couple of warnings were uncovered in#6655
This should take care of the easy ones.

The larger problem is the ctype warnings where we have a bit of a mix between using the properisalnum() and rolling our own withgit__isalnum()
The posix versions take an integer in to be able to handle EOF fromfgetc() so when a char is passed it has to be casted to unsigned so that 128-255 isn't extended to negative integers.
Thegit__isalnum() function takes care of it by simply not handling characters outside of the ascii set and will behave incorrectly on systems using ISO-8859-1 or other charsets using more than 7 bits.

I'm not entirely sure why we have thegit__isalnum() functions. I assume it is to support systems wherectype.h isn't available?
In that case I think the current implementation should be used only when ctype doesn't exists.
There is also the thing where casting characters to unsigned everywhere is a bit tedious, so maybe we should change the functions to take an unsigned char in?

util: make git_futils_readbuffer_updated header match source

51f0b02

Copy link

ContributorAuthor

boretrk commentedNov 20, 2023

Also replaced index() with strchr(). index() isn't POSIX and won't compile on some systems.
I also got some compiler warnings in errors.c because the deprecated stuff weren't pre-declared.

ethomson reviewed

Nov 20, 2023

View reviewed changes

src/util/futils.hShow resolvedHide resolved

src/util/fs_path.c OutdatedShow resolvedHide resolved

src/util/regexp.c OutdatedShow resolvedHide resolved

boretrk added4 commits

November 26, 2023 01:11

util: suppress some uninitialized variable warnings

1f54d5b

util: replace index (deprecated) with strchr

db013b5

util: include git2/deprecated.h in errors.c

3889ea5

In C99 functions need to be declared before they are defined.We could just add the declarations before them, but including theheader allows the compiler to warn if they differ.

util: sudo_uid_lookup should always return error if out isn't set

bde119e

boretrk force-pushed thefixwarn branch from802cd58 tobde119eCompare

November 26, 2023 00:28

Copy link

ContributorAuthor

boretrk commentedNov 26, 2023

OK, I think this one is good to go now.
The character classification stuff will probably have to be its own thing.

Thegit__isalpha() stuff seems to have been added in0f49200 to solve issues with the config parsing so it should probably be left alone?
Over all it seems like we need a set of character type functions that behaves as if locale is set to"C" but is this really always the case?
So when is it correct to use locale sensitive character type classification and when shall we use the git__xxxx() versions?

For reference: When locale is set to "C" we get ISO/IEC 9899:1999 5.2.1 Character sets:

Both the basic source and basic execution character sets shall have the following members: the 26 uppercase letters of the Latin alphabet
A B C D E F G H I J K L M N O P Q R S T U V W X Y Z
the 26 lowercase letters of the Latin alphabet
a b c d e f g h i j k l m n o p q r s t u v w x y z
the 10 decimal digits
0 1 2 3 4 5 6 7 8 9
the following 29 graphic characters
! " # % & ' ( ) * + , - . / : ; < = > ? [ \ ] ^ _ { | } ~
the space character, and control characters representing horizontal tab, vertical tab, and form feed.