Re: GPGMEPP and C++ antipatterns

Ingo Klöcker <[email protected]> Fri, 07 Nov 2025 10:02:53 +0100
Newsgroups	gmane.comp.encryption.gpg.devel
Message-ID	<3583157.sQuhbGJ8Bu@daneel>
Hi Robert,

thanks for your email. Let's move this to the [email protected] mailing 
list. I hope I'm using the correct email address of yours. mailman has 
replaced your address in the From header with "Robert J. Hansen via Gnupg-
users" <[email protected]> (probably because of SPF & friends).

On Donnerstag, 6. November 2025 20:53:26 Mitteleuropäische Normalzeit Robert 
J. Hansen via Gnupg-users wrote:
> After using GPGMEPP for a week or so, I'm pleased with it. Somebody 
> clearly put some thought into how to make it a properly C++ library, 
> rather than just a thin wrapper around a C one. Whoever's responsible 
> for that (Ingo?), thank you.

Thanks, but that was long before I took over.

> However, I do have a couple of minor nits. (Of course I do. It's me.)

Believe me, I have more than just a couple of minor nits about the API. The 
problem is that we do guarantee ABI stability which makes it nigh to 
impossible to fix errors of the past or to modernize the API.

> First, a number of functions accept unsigned ints as a parameter. This 
> involves a minor pain point for those of us working in environments that 
> require us to follow the MISRA C++ guidelines. Admittedly, it's a 
> one-letter fix:
> 
> 	(*ctx).setKeyListMode(0);
> 
> 		|
> 
> 	     becomes
> 
> 		|
> 
> 		V
> 	(*ctx).setKeyListMode(0U);
> 
> but it would be nice if we could find some way to avoid one letter of 
> syntactic sugar and let us express code in the most natural way.
> 
> Second, MISRA has … I can only call them _opinions_, shall we say … on 
> the subject of pointers. Look at, e.g., creating a new Context:
> 
> 	auto ctx = unique_ptr<Context>(OpenPGP);
> 	if (nullptr == ctx) {
> 	    // handle the error
> 	}

See below for Robert correcting the above example.

> Here there are two problems. The first is that GPGMEPP is using 
> old-style enums rather than modern C++ class enums, which means they're 
> not typesafe and it's harder for static analysis tools to detect when 
> you're feeding in garbage.

Looking at the history the first registered commit in the current gpgmepp repo 
is from early 2004, i.e. almost a decade before the modernization of C++ 
started. (The commits that predate this date are lost in an old branch that's 
probably still available in some backup.)

Regarding class enums, I'm not yet sure how/when to use them properly. In my 
understanding they are unsuitable for bit flags and many of gpgmepp enums are 
bit flags.

> The second is that per MISRA, unique_ptrs and 
> shared_ptrs should be created only by calls to make_unique and/or 
> make_shared, not by direct application of the constructor.

I wholeheartedly agree with you and MISRA. But wait: There's
static std::unique_ptr<Context> create(Protocol proto);
since many years. What's missing is a function that returns a shared_ptr.

> Hence, two more suggestions. First, replace all enums with C++ class 
> enums, and second, make createForProtocol take a template parameter of 
> the type of pointer to return, whether unique, shared, or raw.

I'd appreciate if you would propose patches for this. I'm not sure how we can 
make those changes API- and ABI-compatible.

For "pure" enums (like Protocol) the problem is that converting them to "class 
enums" makes them, by definition, scoped which is ABI-compatible but not source 
compatible.

For enums that are used as bit flags I'm at a loss how class enums and bit flags 
can work. The internet seems to suggest that one should simply overload 
operator| to avoid the compiler's complaints, but to me (and others) this 
feels like forcing something to fit which doesn't fit.

For createForProtocol/create the easiest would probably be a new method with a 
different name that replaces the old methods. Too bad that the best name 
"create" is already taken. What about adding a non-templated createShared 
method to complement the existing createForProtocol and create? For version 
3.0 (which I'm not sure will ever happen) we could then clean this up. I'm 
open for alternative suggestions, i.e. we can still add a templated variant if 
you have a good suggestion for a name.

Regarding the implementation of create we can easily make create use 
make_unique internally.

> These minor problems aren't creating any obstacles to my development, 
> just requiring me to fill out a small amount of paperwork documenting 
> the deviations from MISRA. All in all I quite like GPGMEPP. Thanks for 
> the code, guys. :)

Thanks! It's nice to hear that it's useful for others.

On Donnerstag, 6. November 2025 22:00:52 Mitteleuropäische Normalzeit Robert 
J. Hansen via Gnupg-users wrote:
> >      auto ctx = unique_ptr<Context>(OpenPGP);
> 
> 
> Gah! I was literally looking at my code while copying it and still 
> somehow managed to goof it.
> 
> "auto ctx = unique_ptr<Context>(Context::createForProtocol(OpenPGP));"

Regards,
Ingo

_______________________________________________
Gnupg-devel mailing list
[email protected]
https://lists.gnupg.org/mailman/listinfo/gnupg-devel
signature.asc (application/pgp-signature, 265 B)