Skip to content

Removal of old card drivers #2885

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.

Sign up for GitHub

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

Jump to bottom
Merged
Jakuje merged 5 commits into from
Jan 26, 2024
Merged

Removal of old card drivers #2885

Jakuje merged 5 commits into from
Jan 26, 2024

Conversation

@xhanulik
Copy link
Member

@xhanulik xhanulik commented Oct 3, 2023

This PR proposes a removal of old card drivers with no recent user or developer activity. All of the following drivers were disabled in #2391.
The process of disabling and removing of old drivers is described on wiki.

The discussed drivers are:

  • card-akis.c, last developer modification f947614 (2007),
  • card-flex.c, 7d935df (2010),
  • card-gpk.c, a8908b8 (2007),
  • card-incrypto34.c, a2f622a (2007),
  • card-westcos.c, c3de15d (2010).

The decision regarding whether all or some of the drivers should be removed, or if they should remain disabled, is open for discussion.

@frankmorgner
Copy link
Member

frankmorgner commented Oct 4, 2023

Before removing the driver completely, we should put it into the list of old card drivers first.

Also, for GPK there has recently been user activity (#2832), so we should still keep this in the list of old drivers

@xhanulik
Copy link
Member Author

xhanulik commented Oct 4, 2023

Before removing the driver completely, we should put it into the list of old card drivers first.

All of the mentioned drivers were disabled in #2391, so they are on the list of old card drivers:

OpenSC/src/libopensc/ctx.c

Lines 170 to 179 in 33dd3a2

static const struct _sc_driver_entry old_card_drivers[] = {
{ "akis", (void *(*)(void)) sc_get_akis_driver },
{ "asepcos", (void *(*)(void)) sc_get_asepcos_driver },
{ "atrust-acos",(void *(*)(void)) sc_get_atrust_acos_driver },
{ "flex", (void *(*)(void)) sc_get_cryptoflex_driver },
#ifdef ENABLE_OPENSSL
{ "gpk", (void *(*)(void)) sc_get_gpk_driver },
#endif
{ "incrypto34", (void *(*)(void)) sc_get_incrypto34_driver },
{ "westcos", (void *(*)(void)) sc_get_westcos_driver },

Also, for GPK there has recently been user activity (#2832), so we should still keep this in the list of old drivers

I think this issue was regarding the disabled asepcos driver, which is not part of this PR (and the user activity should be already updated on wiki).

@frankmorgner
Copy link
Member

frankmorgner commented Oct 5, 2023

Thanks for the clearification. However, it looks like card-flex.c was still reachable via the "cyberflex" driver, because only the predecessor Cryptoflex (flex) was moved to the old drivers previously. I think it would be nice to keep both in the list of old drivers for an other release cycle. The removal of the other drivers could be scheduled when the final release of 0.24.0 is out.

@xhanulik
Copy link
Member Author

xhanulik commented Oct 5, 2023

Alright, I will move cyberflex into old drivers then.

@xhanulik xhanulik mentioned this pull request Jan 12, 2024
Merged
@xhanulik
Remove support for AKIS cards
75f5df6 
Remove due to no user and developer activity.
Last relevant modification was f947614
and no more changes except general were made.
The driver was disabled in OpenSC#2391.
@xhanulik
Remove support for GPK cards
fc215b9 
Remove due to no user and developer activity.
Last relevant modification was a8908b8
and no more changes except general were made.
The driver was disabled in OpenSC#2391.
@xhanulik
Remove support for Incrypto34 cards
3b29407 
Remove due to no user and developer activity.
Last relevant modification was a2f622a
and no more changes except general were made.
The driver was disabled in OpenSC#2391.
@xhanulik
Remove support for Westcos cards
125cc95 
Remove due to no user and developer activity.
Last relevant modification was c3de15d
and no more changes except general were made.
The driver was disabled in OpenSC#2391.
@xhanulik
Disable cyberflex driver
a49daa7 
Due to no recent user or developer activity
frankmorgner
frankmorgner approved these changes Jan 23, 2024
View reviewed changes
Copy link
Member

@frankmorgner frankmorgner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

libopensc will be ABI incompatible to previous versions due to the change in defines, we need to make sure to change the library version accordingly before a release.

@Jakuje
Copy link
Member

Jakuje commented Jan 24, 2024

Good point! We should probably note it somewhere or start a draft release issue/pr with the notes so it wont get lost.

Just to be clear, are we good to include this with the release in 2 months? I would say yes and lets merge this soon so we can get feedback early if somebody will miss these.

@frankmorgner
Copy link
Member

frankmorgner commented Jan 26, 2024

Good point! We should probably note it somewhere or start a draft release issue/pr with the notes so it wont get lost.

I extended the note for the release how to in the wiki, I think that should be enough:

configure.ac : Update the LT version number, which is required with changes to, for example, opensc.h and libopensc.exports.

Just to be clear, are we good to include this with the release in 2 months? I would say yes and lets merge this soon so we can get feedback early if somebody will miss these.

Yes, good for me.

@Jakuje Jakuje mentioned this pull request Jan 26, 2024
Merged
3 tasks
@Jakuje Jakuje merged commit a6c83ab into OpenSC:master Jan 26, 2024
@Jakuje Jakuje mentioned this pull request Feb 5, 2024
Merged
5 tasks
@xhanulik xhanulik mentioned this pull request Feb 6, 2024
Closed
@xhanulik xhanulik deleted the old-drivers branch July 19, 2024 16:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@frankmorgner frankmorgner frankmorgner approved these changes

@Jakuje Jakuje Jakuje approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@xhanulik @frankmorgner @Jakuje