Skip to content

bpo-34555: Fix incorrectly nested test for HAVE_LINUX_VM_SOCKETS_H #9016

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

Merged
benjaminp merged 3 commits into from
Sep 1, 2018
Merged

bpo-34555: Fix incorrectly nested test for HAVE_LINUX_VM_SOCKETS_H #9016

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4518d4f
fix incorrectly nested test for HAVE_LINUX_VM_SOCKETS_H
Aug 31, 2018
34ca216
add news entry
mcduke Aug 31, 2018
c4a8cfd
add balancing newline at start of ifdef block
mcduke Sep 1, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions Misc/NEWS.d/next/Build/2018-08-31-19-41-09.bpo-34555.dfQcnm.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Fix for case where it was not possible to have both
``HAVE_LINUX_VM_SOCKETS_H`` and ``HAVE_SOCKADDR_ALG`` be undefined.
49 changes: 25 additions & 24 deletions Modules/socketmodule.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,39 +101,40 @@ typedef int socklen_t;
#include <sys/kern_control.h>
#endif

#ifdef HAVE_SOCKADDR_ALG
#include <linux/if_alg.h>
#ifndef AF_ALG
#define AF_ALG 38
#endif
#ifndef SOL_ALG
#define SOL_ALG 279
#endif

#ifdef HAVE_LINUX_VM_SOCKETS_H
# include <linux/vm_sockets.h>
#else
# undef AF_VSOCK
#endif

#ifdef HAVE_SOCKADDR_ALG

@benjaminp benjaminp Sep 1, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It would help to have a space after this line to make it symmetric with the line before the #endif.

@mcduke mcduke Sep 1, 2018

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This makes sense, amended my pull request.


# include <linux/if_alg.h>
# ifndef AF_ALG
# define AF_ALG 38
# endif
# ifndef SOL_ALG
# define SOL_ALG 279
# endif

@benjaminp benjaminp Sep 1, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

These ifdefs seem rather silly to me. If linux/if_alg.h exists, surely sys/socket.h is going to define AF_ALG and SOL_ALG, right?

@mcduke mcduke Sep 1, 2018

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I don't know enough about this topic to be sure, but from a cursory search it appears to be a guideline in the Linux docs to include this block, in case of older header files that don't yet define these constants themselves.


/* Linux 3.19 */
#ifndef ALG_SET_AEAD_ASSOCLEN
#define ALG_SET_AEAD_ASSOCLEN 4
#endif
#ifndef ALG_SET_AEAD_AUTHSIZE
#define ALG_SET_AEAD_AUTHSIZE 5
#endif
# ifndef ALG_SET_AEAD_ASSOCLEN
# define ALG_SET_AEAD_ASSOCLEN 4
# endif
# ifndef ALG_SET_AEAD_AUTHSIZE
# define ALG_SET_AEAD_AUTHSIZE 5
# endif
/* Linux 4.8 */
#ifndef ALG_SET_PUBKEY
#define ALG_SET_PUBKEY 6
#endif
# ifndef ALG_SET_PUBKEY
# define ALG_SET_PUBKEY 6
# endif

#ifndef ALG_OP_SIGN
#define ALG_OP_SIGN 2
#endif
#ifndef ALG_OP_VERIFY
#define ALG_OP_VERIFY 3
#endif
# ifndef ALG_OP_SIGN
# define ALG_OP_SIGN 2
# endif
# ifndef ALG_OP_VERIFY
# define ALG_OP_VERIFY 3
# endif

#endif /* HAVE_SOCKADDR_ALG */

Expand Down