Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
71 changes: 57 additions & 14 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,36 +2,79 @@ name: Build test
on:
push:
branches:
- master
- main
pull_request:
branches:
- master
- main

jobs:
build:
runs-on: ubuntu-latest

steps:
- uses: actions/checkout@v3

- name: Install Dependencies
run:
sudo apt install
libreadline6-dev
autoconf
automake
run: |
sudo apt update -y
sudo apt install -y \
libreadline6-dev \
autoconf \
automake \
python3-pytest \
valgrind

- name: Prepare (autoreconf)
run: autoreconf -i

- name: Prepare (configure)
run: ./configure
# Build 1: ASAN
- name: Create ASAN build directory
run: mkdir -p build-asan

- name: Configure ASAN build
working-directory: build-asan
env:
CFLAGS: "-g -O2 -Wpedantic -Werror -Wno-error=unused-result -fsanitize=address,undefined -fno-omit-frame-pointer"
LDFLAGS: "-fsanitize=address,undefined"
run: ../configure

- name: Build
- name: Build ASAN
working-directory: build-asan
run: make

- name: Run check
- name: Run pytest with ASAN
continue-on-error: true
env:
ASAN_OPTIONS: "detect_leaks=1"
UBSAN_OPTIONS: "print_stacktrace=1"
run: pytest -v --cmd="build-asan/microcom"

- name: Run check with ASAN
continue-on-error: true
working-directory: build-asan
env:
ASAN_OPTIONS: "detect_leaks=1"
UBSAN_OPTIONS: "print_stacktrace=1"
run: make check

- name: Run distcheck
run: make distcheck
# Build 2: Valgrind
- name: Create unoptimized build directory
run: mkdir -p build-unopt

- name: Configure unoptimized build
working-directory: build-unopt
env:
CFLAGS: "-g -O0 -Wpedantic -Werror -Wno-error=unused-result"
run: ../configure

- name: Build unoptimized
working-directory: build-unopt
run: make

- name: Run pytest with Valgrind
continue-on-error: true
run: pytest -v --cmd="valgrind --tool=memcheck --error-exitcode=99 --quiet build-unopt/microcom"

- name: Run check with Valgrind
continue-on-error: true
working-directory: build-unopt
run: make check
21 changes: 21 additions & 0 deletions .github/workflows/style.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
name: Enforce coding style
on:
pull_request:
branches:
- main

jobs:
style:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v3

- name: Install Dependencies
run: |
sudo apt update -y
sudo apt install -y \
uncrustify
- name: enforce style using uncrustify
run: |
uncrustify --replace -c uncrustify.cfg *.c *.h
git diff --exit-code
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
*~
/*.o
/microcom
*.pyc

# autotools stuff
/.deps
Expand Down
4 changes: 2 additions & 2 deletions can.c
Original file line number Diff line number Diff line change
Expand Up @@ -184,8 +184,8 @@ struct ios_ops *can_init(char *interface_id)
return NULL;
}

printf("connected to %s (rx_id=%x, tx_id=%x)\n",
interface, filter->can_id, data.can_id);
msg_printf("connected to %s (rx_id=%x, tx_id=%x)\n",
interface, filter->can_id, data.can_id);

return ios;
}
104 changes: 55 additions & 49 deletions microcom.c
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ static struct termios sots; /* old stdout/in termios settings to restore */

struct ios_ops *ios;
int debug;
int quiet;
Copy link
Member

Choose a reason for hiding this comment

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

#55 explicitly zeros debug. Not zeroing quiet now aligns it with the code as-is, but since #55 should be merged first I think this should be: int quiet = 0;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes it's an artifact from my attempt to split up what was originally a single series. quiet is zero-initialized on the "single series" branch to which I can update this pr once the dependent PRs are merged.

I suppose unitialized globals are more a bad style practice than actual reliance on unitialized memory, at least on linux where they're zero-initialized as part of the bss (in contrast to locals which are stack-allocated and not automatically initialized).

Anyway, yes this will be taken care of :)


void init_terminal(void)
{
Expand Down Expand Up @@ -98,6 +99,7 @@ void main_usage(int exitcode, char *str, char *dev)
" default: (%s:%x:%x)\n"
" -f, --force ignore existing lock file\n"
" -d, --debug output debugging info\n"
" -q, --quiet do not print status information to stdout\n"
" -l, --logfile=<logfile> log output to <logfile>\n"
" -o, --listenonly Do not modify local terminal, do not send input\n"
" from stdin\n"
Expand Down Expand Up @@ -136,6 +138,7 @@ int main(int argc, char *argv[])
{ "telnet", required_argument, NULL, 't'},
{ "can", required_argument, NULL, 'c'},
{ "debug", no_argument, NULL, 'd' },
{ "quiet", no_argument, NULL, 'q' },
{ "force", no_argument, NULL, 'f' },
{ "logfile", required_argument, NULL, 'l'},
{ "listenonly", no_argument, NULL, 'o'},
Expand All @@ -144,54 +147,57 @@ int main(int argc, char *argv[])
{ },
};

while ((opt = getopt_long(argc, argv, "hp:s:t:c:dfl:oi:a:e:v", long_options, NULL)) != -1) {
while ((opt = getopt_long(argc, argv, "hp:s:t:c:dqfl:oi:a:e:v", long_options, NULL)) != -1) {
switch (opt) {
case '?':
main_usage(1, "", "");
break;
case 'h':
main_usage(0, "", "");
break;
case 'v':
printf("%s\n", PACKAGE_VERSION);
exit(EXIT_SUCCESS);
break;
case 'p':
device = optarg;
break;
case 's':
current_speed = strtoul(optarg, NULL, 0);
break;
case 't':
telnet = 1;
hostport = optarg;
break;
case 'c':
can = 1;
interfaceid = optarg;
break;
case 'f':
opt_force = 1;
break;
case 'd':
debug = 1;
break;
case 'l':
logfile = optarg;
break;
case 'o':
listenonly = 1;
break;
case 'a':
answerback = optarg;
break;
case 'e':
if (strlen(optarg) != 1) {
fprintf(stderr, "Option -e requires a single character argument.\n");
exit(EXIT_FAILURE);
}
escape_char = *optarg;
break;
case '?':
main_usage(1, "", "");
break;
case 'h':
main_usage(0, "", "");
break;
case 'v':
printf("%s\n", PACKAGE_VERSION);
exit(EXIT_SUCCESS);
break;
case 'p':
device = optarg;
break;
case 's':
current_speed = strtoul(optarg, NULL, 0);
break;
case 't':
telnet = 1;
hostport = optarg;
break;
case 'c':
can = 1;
interfaceid = optarg;
break;
case 'f':
opt_force = 1;
break;
case 'd':
debug = 1;
break;
case 'q':
quiet = 1;
break;
case 'l':
logfile = optarg;
break;
case 'o':
listenonly = 1;
break;
case 'a':
answerback = optarg;
break;
case 'e':
if (strlen(optarg) != 1) {
fprintf(stderr, "Option -e requires a single character argument.\n");
exit(EXIT_FAILURE);
}
escape_char = *optarg;
break;
Comment on lines +152 to +200
Copy link
Member

Choose a reason for hiding this comment

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

This mixes a formatting change with a code change. It would be great to only have the

+		case 'q':
+			quiet = 1;
+			break;

In this commit.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I originally based this on all the three dependent branches and only haphazardly rebased to main to avoid making too large a pull request.
The diff here will be more meaningful once #55 is closed. I'm converting this pr to draft until then.

}
}

Expand Down Expand Up @@ -239,8 +245,8 @@ int main(int argc, char *argv[])
ios->set_flow(ios, current_flow);

if (!listenonly) {
printf("Escape character: Ctrl-%c\n", escape_char);
printf("Type the escape character to get to the prompt.\n");
msg_printf("Escape character: Ctrl-%c\n", escape_char);
msg_printf("Type the escape character to get to the prompt.\n");

/* Now deal with the local terminal side */
tcgetattr(STDIN_FILENO, &sots);
Expand Down
4 changes: 3 additions & 1 deletion microcom.h
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ void main_usage(int exitcode, char *str, char *dev);

extern struct ios_ops *ios;
extern int debug;
extern int quiet;
extern int opt_force;
extern int listenonly;
extern char *answerback;
Expand Down Expand Up @@ -121,7 +122,8 @@ extern int current_flow;
int do_commandline(void);
int do_script(char *script);

#define dbg_printf(fmt,args...) ({ if (debug) printf(fmt ,##args); })
#define dbg_printf(...) do { if (debug) printf(__VA_ARGS__); } while (0)
#define msg_printf(...) do { if (!quiet) printf(__VA_ARGS__); } while (0)

/*
* Some telnet options according to
Expand Down
2 changes: 1 addition & 1 deletion serial.c
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,7 @@ struct ios_ops * serial_init(char *device)
memcpy(&pots, &pts, sizeof (pots));
init_comm(&pts);
tcsetattr(fd, TCSANOW, &pts);
printf("connected to %s\n", device);
msg_printf("connected to %s\n", device);

return ops;
}
Expand Down
2 changes: 1 addition & 1 deletion telnet.c
Original file line number Diff line number Diff line change
Expand Up @@ -603,7 +603,7 @@ struct ios_ops *telnet_init(char *hostport)
fprintf(stderr, "getnameinfo: %s\n", gai_strerror(ret));
goto out;
}
printf("connected to %s (port %s)\n", connected_host, connected_port);
msg_printf("connected to %s (port %s)\n", connected_host, connected_port);

/* send intent we WILL do COM_PORT stuff */
dbg_printf("-> WILL COM_PORT_CONTROL\n");
Expand Down
24 changes: 24 additions & 0 deletions test/conftest.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import os
from pathlib import Path
import shlex
import pytest


def pytest_sessionstart(session):
# run tests in toplevel directory always, not in test/
os.chdir(Path(__file__).resolve().parent.parent)


def pytest_addoption(parser):
parser.addoption(
"--cmd",
action="store",
default="./microcom",
help="Command used to invoke microcom"
)


@pytest.fixture(scope="session")
def cmd(pytestconfig):
cmd = pytestconfig.getoption("--cmd")
return shlex.split(cmd)
Loading
Loading