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

Main #94

Merged
merged 1 commit into from
Jun 30, 2023
Merged

Main #94

merged 1 commit into from
Jun 30, 2023

Conversation

THEWWWTHING
Copy link
Contributor

No description provided.

@ydahhrk
Copy link
Member

ydahhrk commented Apr 17, 2023

  1. Why?
  2. It doesn't compile:
$ ./autogen.sh
$ ./configure
$ make
Making all in src
make[1]: Entering directory '/home/aleiva/git/fort/src'
make  all-am
make[2]: Entering directory '/home/aleiva/git/fort/src'
gcc -DHAVE_CONFIG_H -I.    -Wall -Wno-cpp -Wpedantic -std=gnu11 -O2 -g  -I/usr/include/libxml2 -DBACKTRACE_ENABLED -g -O2 -MT fort-main.o -MD -MP -MF .deps/fort-main.Tpo -c -o fort-main.o `test -f 'main.c' || echo './'`main.c
In file included from asn1/asn1c/ASIdOrRange.h:16,
                 from asn1/asn1c/ASIdentifierChoice.h:61,
                 from asn1/asn1c/ASIdentifiers.h:41,
                 from resource.h:7,
                 from cert_stack.h:6,
                 from state.h:5,
                 from thread_var.h:4,
                 from main.c:6:
asn1/asn1c/ASRange.h:24:2: error: unknown type name ‘ASId_t’
   24 |  ASId_t  min;
      |  ^~~~~~
asn1/asn1c/ASRange.h:25:2: error: unknown type name ‘ASId_t’
   25 |  ASId_t  max;
      |  ^~~~~~
In file included from asn1/asn1c/ASIdentifierChoice.h:61,
                 from asn1/asn1c/ASIdentifiers.h:41,
                 from resource.h:7,
                 from cert_stack.h:6,
                 from state.h:5,
                 from thread_var.h:4,
                 from main.c:6:
asn1/asn1c/ASIdOrRange.h:34:3: error: unknown type name ‘ASId_t’
   34 |   ASId_t  id;
      |   ^~~~~~
make[2]: *** [Makefile:1690: fort-main.o] Error 1
make[2]: Leaving directory '/home/aleiva/git/fort/src'
make[1]: *** [Makefile:867: all] Error 2
make[1]: Leaving directory '/home/aleiva/git/fort/src'
make: *** [Makefile:362: all-recursive] Error 1

Copy link
Contributor Author

@THEWWWTHING THEWWWTHING left a comment

Choose a reason for hiding this comment

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

Thx

@ydahhrk
Copy link
Member

ydahhrk commented Jun 26, 2023

Wait... I just noticed we have two AS identifiers (ASID.h and ASId.h), and they're basically identical.

Is this giving you trouble in a case-insensitive file system? If so, it would seem to me the proper solution would be to delete one of them, not make them fully identical.

@ydahhrk ydahhrk closed this in 84952a9 Jun 30, 2023
@ydahhrk ydahhrk merged commit 26e9760 into NICMx:main Jun 30, 2023
@ydahhrk
Copy link
Member

ydahhrk commented Jun 30, 2023

Ok, I suppose you're not going to elaborate further at this point, so I fixed what I thought was broken, okay. Hopefully it'll solve whatever problem you're having.

@Irsservices
Copy link

Irsservices commented Jul 2, 2023 via email

@ydahhrk
Copy link
Member

ydahhrk commented Jul 3, 2023

As proposed, THEWWWTHING's patch turned the ASId.h module into an exact clone of the ASID.h module.

Whatever the goal was, the patch was misguided because none of the ASId usages were updated, so the code did not compile.

While reviewing the patch, I realized there was no reason to keep ASId and ASID separate. They were the same type, only spelled a little differently. My theory is that THEWWWTHING downloaded the code into a filesystem which, because of file name collision, wasn't capable of containing both files (ASId.h and ASID.h) in the same folder. So one thing lead to another and they ended up uploading a pull request in which both files had the same content.

I fixed the pull request by fusing the two types. The issue is now closed because I already merged the patch into main.

You can download the latest version of the code by clicking <> Code > Download ZIP here. (Or just git cloning as usual.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants