feat: add `.clang-format` #120

saurabhraghuvanshii · 2025-02-24T15:31:52Z

fixes #104

Add .clang-format for format file with some set rule
And To keep consistent format
refer some pre built style file ( WebKit , LLVM, Microsoft)

Test

test locally it work on file by running command clang-format -i <filename>

Next step

for automation on CI , falling if code is not formatted
And documentation in CONTRIBUTING.md

jandubois · 2025-02-24T19:16:04Z

@saurabhraghuvanshii Can you please update your commits to include a DCO (see https://github.com/lima-vm/socket_vmnet/pull/120/checks?check_run_id=37732521320 for more details)

@nirs Can you do a review of the .clang-format file to see if that is what we want?

saurabhraghuvanshii · 2025-02-24T19:18:18Z

I will sir update my commit for now my wifi is not working and my mobile data is not sufficient.

.github/workflows/clang-format.yaml

saurabhraghuvanshii · 2025-02-24T19:25:41Z

Okay sir, I will update I am thinking of asking this. But for clear i create new file. Thanks sir

nirs

I'll review more later, added some comments for now.

.clang-format

.github/workflows/clang-format.yaml

.github/workflows/build.yaml

nirs

The change looks simple now, but there is no reason to split to several commits. Please squash the commit to one commit.

.clang-format

.github/workflows/build.yaml

nirs · 2025-04-02T22:11:21Z

Lets try to do this work smaller steps:

Create a .clang-format file that keep the code mostly unchanged when formatting it.
Modify the code so it matches the clang-format file
Add a clang-format job that format all code and fail if the diff is not empty.

nirs · 2025-04-02T22:30:35Z

If we use LLVM style the code needs only minor changes:

$ cat .clang-format 
---
Language: Cpp
BasedOnStyle:  LLVM
---

$ clang-format -i *.c *.h client/*.c

$ git diff --stat                   
 cli.c  | 30 ++++++++++++++++--------------
 log.h  | 12 ++++++------
 main.c | 56 ++++++++++++++++++++++++++++----------------------------
 3 files changed, 50 insertions(+), 48 deletions(-)

The diff is much smaller and include simple modifications that makes sense. The following are example changes from the diff.

This is uglier - but it is easier to work with the code if we don't need to format it manually:

   const struct option longopts[] = {
-    {"socket-group",        required_argument,  NULL,   CLI_OPT_SOCKET_GROUP},
-    {"vmnet-mode",          required_argument,  NULL,   CLI_OPT_VMNET_MODE},
-    {"vmnet-interface",     required_argument,  NULL,   CLI_OPT_VMNET_INTERFACE},
-    {"vmnet-gateway",       required_argument,  NULL,   CLI_OPT_VMNET_GATEWAY},
-    {"vmnet-dhcp-end",      required_argument,  NULL,   CLI_OPT_VMNET_DHCP_END},
-    {"vmnet-mask",          required_argument,  NULL,   CLI_OPT_VMNET_MASK},
-    {"vmnet-interface-id",  required_argument,  NULL,   CLI_OPT_VMNET_INTERFACE_ID},
-    {"vmnet-nat66-prefix",  required_argument,  NULL,   CLI_OPT_VMNET_NAT66_PREFIX},
-    {"pidfile",             required_argument,  NULL,   'p'},
-    {"help",                no_argument,        NULL,   'h'},
-    {"version",             no_argument,        NULL,   'v'},
-    {0, 0, 0, 0},
+      {"socket-group", required_argument, NULL, CLI_OPT_SOCKET_GROUP},
+      {"vmnet-mode", required_argument, NULL, CLI_OPT_VMNET_MODE},
+      {"vmnet-interface", required_argument, NULL, CLI_OPT_VMNET_INTERFACE},
+      {"vmnet-gateway", required_argument, NULL, CLI_OPT_VMNET_GATEWAY},
+      {"vmnet-dhcp-end", required_argument, NULL, CLI_OPT_VMNET_DHCP_END},
+      {"vmnet-mask", required_argument, NULL, CLI_OPT_VMNET_MASK},
+      {"vmnet-interface-id", required_argument, NULL,
+       CLI_OPT_VMNET_INTERFACE_ID},
+      {"vmnet-nat66-prefix", required_argument, NULL,
+       CLI_OPT_VMNET_NAT66_PREFIX},
+      {"pidfile", required_argument, NULL, 'p'},
+      {"help", no_argument, NULL, 'h'},
+      {"version", no_argument, NULL, 'v'},
+      {0, 0, 0, 0},
   };
   int opt = 0;
   while ((opt = getopt_long(argc, argv, "hvp:", longopts, NULL)) != -1) {

Aligning to starting "(" looks good, we can add a commit with these changes.

@@ -196,7 +198,7 @@ struct cli_options *cli_options_parse(int argc, char *argv[]) {
   if (res->vmnet_gateway == NULL) {
     if (res->vmnet_mode != VMNET_BRIDGED_MODE) {
       WARN("--vmnet-gateway=IP should be explicitly specified to "
-          "avoid conflicting with other applications");
+           "avoid conflicting with other applications");
     }
     if (res->vmnet_dhcp_end != NULL) {
       ERROR("--vmnet-dhcp-end=IP requires --vmnet-gateway=IP");

Again the aligned version looks better, but it is easier to maintain the clang-format style:

-#define INFOF(fmt, ...)     fprintf(stderr, "INFO | " fmt "\n", __VA_ARGS__)
-#define ERROR(msg)          fprintf(stderr, "ERROR| " msg "\n")
-#define ERRORF(fmt, ...)    fprintf(stderr, "ERROR| " fmt "\n", __VA_ARGS__)
-#define ERRORN(name)        ERRORF(name ": %s", strerror(errno))
-#define WARN(msg)           fprintf(stderr, "WARN | " msg "\n")
-#define WARNF(fmt, ...)     fprintf(stderr, "WARN | " fmt "\n", __VA_ARGS__)
+#define INFOF(fmt, ...) fprintf(stderr, "INFO | " fmt "\n", __VA_ARGS__)
+#define ERROR(msg) fprintf(stderr, "ERROR| " msg "\n")
+#define ERRORF(fmt, ...) fprintf(stderr, "ERROR| " fmt "\n", __VA_ARGS__)
+#define ERRORN(name) ERRORF(name ": %s", strerror(errno))
+#define WARN(msg) fprintf(stderr, "WARN | " msg "\n")
+#define WARNF(fmt, ...) fprintf(stderr, "WARN | " fmt "\n", __VA_ARGS__)

This is clearly better, we can add a commit adding these changes:

 #endif /* SOCKET_VMNET_LOG_H */
diff --git a/main.c b/main.c
index a2e87de..2752eec 100644
--- a/main.c
+++ b/main.c
@@ -110,7 +110,7 @@ static void state_remove_socket_fd(struct state *state, int socket_fd) {
       for (conn = state->conns; conn->next != NULL; conn = conn->next) {
         if (conn->next->socket_fd == socket_fd) {
           conn->next = conn->next->next;
-         break;
+          break;
         }
       }
     }

This is not better it is consistent with the line width. We can increase the line width to avoid such changes.

@@ -219,7 +219,8 @@ static void on_vmnet_packets_available(interface_ref iface, int64_t estim_count,
          "r=%lld",
          estim_count, MAX_PACKET_COUNT_AT_ONCE, q, r);
   for (int i = 0; i < q; i++) {
-    _on_vmnet_packets_available(iface, MAX_PACKET_COUNT_AT_ONCE, max_bytes, state);
+    _on_vmnet_packets_available(iface, MAX_PACKET_COUNT_AT_ONCE, max_bytes,
+                                state);
   }
   if (r > 0)
     _on_vmnet_packets_available(iface, r, max_bytes, state);

This looks worse. The fix is to create a helper function and avoid the excessive nesting in the current code.

@@ -256,16 +257,17 @@ static interface_ref start(struct state *state, struct cli_options *cliopt) {
   __block vmnet_return_t status;
 
   __block uint64_t max_bytes = 0;
-  iface = vmnet_start_interface(
-      dict, state->host_queue, ^(vmnet_return_t x_status, xpc_object_t x_param) {
-        status = x_status;
-        if (x_status == VMNET_SUCCESS) {
-          print_vmnet_start_param(x_param);
-          max_bytes =
-              xpc_dictionary_get_uint64(x_param, vmnet_max_packet_size_key);
-        }
-        dispatch_semaphore_signal(sem);
-      });
+  iface =
+      vmnet_start_interface(dict, state->host_queue,
+                            ^(vmnet_return_t x_status, xpc_object_t x_param) {
+                              status = x_status;
+                              if (x_status == VMNET_SUCCESS) {
+                                print_vmnet_start_param(x_param);
+                                max_bytes = xpc_dictionary_get_uint64(
+                                    x_param, vmnet_max_packet_size_key);
+                              }
+                              dispatch_semaphore_signal(sem);
+                            });
   dispatch_semaphore_wait(sem, DISPATCH_TIME_FOREVER);
   xpc_release(dict);
   if (status != VMNET_SUCCESS) {

This seems to fix the style to match other code, so we can take it.

-static void remove_pidfile(const char *pidfile)
-{
+static void remove_pidfile(const char *pidfile) {
   if (unlink(pidfile) != 0) {
     ERRORF("Failed to remove pidfile: \"%s\": %s", pidfile, strerror(errno));
     return;
@@ -362,8 +363,7 @@ static void remove_pidfile(const char *pidfile)
   INFOF("Removed pidfile \"%s\" for process %d", pidfile, getpid());
 }

Not sure why this uses more indentation but it looks good.

-static int setup_signals(int kq)
-{
+static int setup_signals(int kq) {
   struct kevent changes[] = {
-    { .ident=SIGHUP, .filter=EVFILT_SIGNAL, .flags=EV_ADD },
-    { .ident=SIGINT, .filter=EVFILT_SIGNAL, .flags=EV_ADD },
-    { .ident=SIGTERM, .filter=EVFILT_SIGNAL, .flags=EV_ADD },
+      {.ident = SIGHUP, .filter = EVFILT_SIGNAL, .flags = EV_ADD},
+      {.ident = SIGINT, .filter = EVFILT_SIGNAL, .flags = EV_ADD},
+      {.ident = SIGTERM, .filter = EVFILT_SIGNAL, .flags = EV_ADD},
   };

This looks fine, we can take this change or increase the line width.

     WARN("Running without root. This is very unlikely to work: See README.md");
   }
   if (geteuid() != getuid()) {
-    WARN("Seems running with SETUID. This is insecure and highly discouraged: See README.md");
+    WARN("Seems running with SETUID. This is insecure and highly discouraged: "
+         "See README.md");
   }

It is easier to work with the older format but the new format is automatic so we can go with the new one.

   // Queue for vm connections, allowing processing vms requests in parallel.
-  state.vms_queue = dispatch_queue_create(
-      "io.github.lima-vm.socket_vmnet.vms", DISPATCH_QUEUE_CONCURRENT);
+  state.vms_queue = dispatch_queue_create("io.github.lima-vm.socket_vmnet.vms",
+                                          DISPATCH_QUEUE_CONCURRENT);

nirs · 2025-04-02T22:32:35Z

@saurabhraghuvanshii I did not try other styles - maybe they match better the current code. It will be good idea to experiment with all existing styles and pick the one that need least amount of changes to the current code.

saurabhraghuvanshii · 2025-04-03T10:03:19Z

@saurabhraghuvanshii I did not try other styles - maybe they match better the current code. It will be good idea to experiment with all existing styles and pick the one that need least amount of changes to the current code

@nirs sir I tried other styles but they don't match much better, LLVM matches most and minimum changes required there I used It.

saurabhraghuvanshii · 2025-04-03T12:00:39Z

I increased line width which is looks goods now , I think now .clang-format match our code . If any changes need I will do thanks for suggestion sir .

nirs · 2025-04-03T12:14:07Z

I increased line width which is looks goods now , I think now .clang-format match our code . If any changes need I will do thanks for suggestion sir .

120 characters is too much and makes it harder to review diffs side by side or use git tools with small shell windows. We cannot assume that everyone is working in full screen mode on a large monitor. It is also bad for readability encouraging long variables names and lot of nesting.

I would keep the default line width for now and do minimal code change to make it work with the default LLVM format. We an consider larger line width in the next step.

.clang-format

saurabhraghuvanshii · 2025-04-03T12:47:02Z

on running this llvm

saurabh@MSI:~/open-source-web/socket_vmnet$ clang-format -i *.c *.h client/*.c
saurabh@MSI:~/open-source-web/socket_vmnet$ git diff --stat
 cli.c         |  40 ++++++++++---------
 client/main.c |   6 +--
 log.h         |  20 +++++-----
 main.c        | 108 +++++++++++++++++++++++++--------------------------
 4 files changed, 88 insertions(+), 86 deletions(-)
saurabh@MSI:~/open-source-web/socket_vmnet$

saurabhraghuvanshii · 2025-04-03T13:20:12Z

removed all unnecessary things from .clang-format , now I have see for arrays align , which I saw in document

.github/workflows/build.yaml

.clang-format

jandubois · 2025-04-04T00:55:38Z

120 characters is too much and makes it harder to review diffs side by side or use git tools with small shell windows. We cannot assume that everyone is working in full screen mode on a large monitor. It is also bad for readability encouraging long variables names and lot of nesting.

I would keep the default line width for now and do minimal code change to make it work with the default LLVM format. We an consider larger line width in the next step.

The default is just 80 columns, which is too short. Nobody uses punched cards or 3270 terminals anymore, and forcing the code into 80 columns results in ridiculous changes like these:

-    _on_vmnet_packets_available(iface, MAX_PACKET_COUNT_AT_ONCE, max_bytes, state);
+    _on_vmnet_packets_available(iface, MAX_PACKET_COUNT_AT_ONCE, max_bytes,
+                                state);

…

-      ERRORF("vmnet_write: [%d] %s", write_status, vmnet_strerror(write_status));
+      ERRORF("vmnet_write: [%d] %s", write_status,
+             vmnet_strerror(write_status));

So maybe 120 is too long, but how about 100?

jandubois · 2025-04-04T01:11:20Z

Okay sir, I will update I am thinking of asking this. But for clear i create new file. Thanks sir

This is informal collaboration on open source, not the military. There is no need to call anybody "sir". Maybe it is a cultural thing, but to me calling people "sir" in issues and PRs feels rather weird.

saurabhraghuvanshii · 2025-04-04T03:59:37Z

120 characters is too much and makes it harder to review diffs side by side or use git tools with small shell windows. We cannot assume that everyone is working in full screen mode on a large monitor. It is also bad for readability encouraging long variables names and lot of nesting.
I would keep the default line width for now and do minimal code change to make it work with the default LLVM format. We an consider larger line width in the next step.

The default is just 80 columns, which is too short. Nobody uses punched cards or 3270 terminals anymore, and forcing the code into 80 columns results in ridiculous changes like these:
-    _on_vmnet_packets_available(iface, MAX_PACKET_COUNT_AT_ONCE, max_bytes, state);
+    _on_vmnet_packets_available(iface, MAX_PACKET_COUNT_AT_ONCE, max_bytes,
+                                state);

…

-      ERRORF("vmnet_write: [%d] %s", write_status, vmnet_strerror(write_status));
+      ERRORF("vmnet_write: [%d] %s", write_status,
+             vmnet_strerror(write_status));
So maybe 120 is too long, but how about 100?

I tried 100 column it also break lines to next like 120.

nirs · 2025-04-04T09:58:11Z

120 characters is too much and makes it harder to review diffs side by side or use git tools with small shell windows. We cannot assume that everyone is working in full screen mode on a large monitor. It is also bad for readability encouraging long variables names and lot of nesting.
I would keep the default line width for now and do minimal code change to make it work with the default LLVM format. We an consider larger line width in the next step.

The default is just 80 columns, which is too short. Nobody uses punched cards or 3270 terminals anymore, and forcing the code into 80 columns results in ridiculous changes like these:
-    _on_vmnet_packets_available(iface, MAX_PACKET_COUNT_AT_ONCE, max_bytes, state);
+    _on_vmnet_packets_available(iface, MAX_PACKET_COUNT_AT_ONCE, max_bytes,
+                                state);

…

-      ERRORF("vmnet_write: [%d] %s", write_status, vmnet_strerror(write_status));
+      ERRORF("vmnet_write: [%d] %s", write_status,
+             vmnet_strerror(write_status));
So maybe 120 is too long, but how about 100?

100 sounds good compromise, long enough to avoid unwanted line breaks and short enough to make it easy to compare side by side diffs and work with many smaller shell windows. This is also the width the Linux kernel chose as a hard limit recently. Adding the limit now will minimize the changes to needed to have automatic formatting.

jandubois

I'm confused by the CI results; how did a Windows path end up in here:

Run # Format all source - fi the source is formatted properly this will do nothing
Configuration file(s) do(es) not support C: /Users/runner/work/socket_vmnet/socket_vmnet/.clang-format
Configuration file(s) do(es) not support C: /Users/runner/work/socket_vmnet/socket_vmnet/.clang-format
Configuration file(s) do(es) not support C: /Users/runner/work/socket_vmnet/socket_vmnet/.clang-format
Error: Process completed with exit code 1.

.clang-format

.github/workflows/build.yaml

nirs · 2025-04-04T17:57:35Z

I'm confused by the CI results; how did a Windows path end up in here:

Run # Format all source - fi the source is formatted properly this will do nothing
Configuration file(s) do(es) not support C: /Users/runner/work/socket_vmnet/socket_vmnet/.clang-format
Configuration file(s) do(es) not support C: /Users/runner/work/socket_vmnet/socket_vmnet/.clang-format
Configuration file(s) do(es) not support C: /Users/runner/work/socket_vmnet/socket_vmnet/.clang-format
Error: Process completed with exit code 1.

This is not windows path, I think it means that the configuration does not support the C language. Maybe because we call it with ".c" instead of ".cpp" or "*.cxx"?

I don't see this locally in macOS 15.3.2.

Lets try this:

brew update
brew install clang-format
clang-format --version

nirs · 2025-04-04T18:19:41Z

brew update
brew install clang-format
clang-format --version

Locally I have:

% clang-format --version
clang-format version 20.1.2

In the CI we install almost the same version:
Pouring clang-format--20.1.1.arm64_sequoia.bottle.tar.gz

So this is probably not the issue.

nirs · 2025-04-04T18:46:04Z

I tested this change in my fork and it works with some minor changes:
https://github.com/nirs/socket_vmnet/actions/runs/14271922180/job/40006878846

Changed .clang-format to :

---
Language: Cpp
BasedOnStyle: LLVM
AlignArrayOfStructures: Left
ColumnLimit: 100

And added proper indentation to the embedded script:

      - name: Check code
        run: |
          # Format all source - fi the source is formatted properly this will do nothing
          clang-format -i *.c *.h client/*.c

          # Do we have unwanted changes?
          if ! git diff-index --quiet HEAD; then
              # Show the changes
              echo "ERROR: Code is not properly formatted:"
              echo
              git --no-pager diff

              echo
              echo >&2 "Please run clang-format locally and commit the changes."
              exit 1
          fi

I don't think this can change the buidl, but it works.

One issue - logging to stdout and stderr does not work - we get mixed output:

ERROR: Code is not properly formatted:

diff --git a/cli.c b/cli.c
index 2226a1e..4aeccc0 100644
--- a/cli.c
+++ b/cli.c
@@ -30,8 +30,7 @@ static void print_usage(const char *argv0) {
   printf("\n");
   printf("--socket-group=GROUP                socket group name (default: "
          "\"" CLI_DEFAULT_SOCKET_GROUP "\")\n");
-  printf(
-      "--vmnet-mode=(host|shared|bridged)  vmnet mode (default: \"shared\")\n");
+  printf("--vmnet-mode=(host|shared|bridged)  vmnet mode (default: \"shared\")\n");
   printf("--vmnet-interface=INTERFACE         interface used for "
          "--vmnet=bridged, e.g., \"en0\"\n");
   printf("--vmnet-gateway=IP                  gateway used for "
@@ -83,[18](https://github.com/nirs/socket_vmnet/actions/runs/14271922180/job/40006878846#step:4:19) +82,18 @@ struct cli_options *cli_options_parse(int argc, char *argv[]) {
   }
 
   const struct option longopts[] = {
-    {"socket-group",        required_argument,  NULL,   CLI_OPT_SOCKET_GROUP},
-    {"vmnet-mode",          required_argument,  NULL,   CLI_OPT_VMNET_MODE},
-    {"vmnet-interface",     required_argument,  NULL,   CLI_OPT_VMNET_INTERFACE},
-    {"vmnet-gateway",       required_argument,  NULL,   CLI_OPT_VMNET_GATEWAY},
-    {"vmnet-dhcp-end",      required_argument,  NULL,   CLI_OPT_VMNET_DHCP_END},
-    {"vmnet-mask",          required_argument,  NULL,   CLI_OPT_VMNET_MASK},
-    {"vmnet-interface-id",  required_argument,  NULL,   CLI_OPT_VMNET_INTERFACE_ID},
-    {"vmnet-nat66-prefix",  required_argument,  NULL,   CLI_OPT_VMNET_NAT66_PREFIX},
-    {"pidfile",             required_argument,  NULL,   'p'},
-    {"help",                no_argument,        NULL,   'h'},
-    {"version",             no_argument,        NULL,   'v'},
-    {0, 0, 0, 0},
+      {"socket-group",       required_argument, NULL, CLI_OPT_SOCKET_GROUP      },
+      {"vmnet-mode",         required_argument, NULL, CLI_OPT_VMNET_MODE        },
+      {"vmnet-interface",    required_argument, NULL, CLI_OPT_VMNET_INTERFACE   },
Please run clang-format locally and commit the changes.
+      {"vmnet-gateway",      required_argument, NULL, CLI_OPT_VMNET_GATEWAY     },
+      {"vmnet-dhcp-end",     required_argument, NULL, CLI_OPT_VMNET_DHCP_END    },
+      {"vmnet-mask",         required_argument, NULL, CLI_OPT_VMNET_MASK        },

We should log everything to stdout or stderr, we cannot use both. Since logging to stderr is harder and, lets use only stdout.

.github/workflows/build.yaml

saurabhraghuvanshii · 2025-04-04T19:02:30Z

I tested this change in my fork and it works with some minor changes: https://github.com/nirs/socket_vmnet/actions/runs/14271922180/job/40006878846

Changed .clang-format to :

---
Language: Cpp
BasedOnStyle: LLVM
AlignArrayOfStructures: Left
ColumnLimit: 100

And added proper indentation to the embedded script:

      - name: Check code
        run: |
          # Format all source - fi the source is formatted properly this will do nothing
          clang-format -i *.c *.h client/*.c

          # Do we have unwanted changes?
          if ! git diff-index --quiet HEAD; then
              # Show the changes
              echo "ERROR: Code is not properly formatted:"
              echo
              git --no-pager diff

              echo
              echo >&2 "Please run clang-format locally and commit the changes."
              exit 1
          fi

I don't think this can change the buidl, but it works.

One issue - logging to stdout and stderr does not work - we get mixed output:

ERROR: Code is not properly formatted:

diff --git a/cli.c b/cli.c
index 2226a1e..4aeccc0 100644
--- a/cli.c
+++ b/cli.c
@@ -30,8 +30,7 @@ static void print_usage(const char *argv0) {
   printf("\n");
   printf("--socket-group=GROUP                socket group name (default: "
          "\"" CLI_DEFAULT_SOCKET_GROUP "\")\n");
-  printf(
-      "--vmnet-mode=(host|shared|bridged)  vmnet mode (default: \"shared\")\n");
+  printf("--vmnet-mode=(host|shared|bridged)  vmnet mode (default: \"shared\")\n");
   printf("--vmnet-interface=INTERFACE         interface used for "
          "--vmnet=bridged, e.g., \"en0\"\n");
   printf("--vmnet-gateway=IP                  gateway used for "
@@ -83,[18](https://github.com/nirs/socket_vmnet/actions/runs/14271922180/job/40006878846#step:4:19) +82,18 @@ struct cli_options *cli_options_parse(int argc, char *argv[]) {
   }
 
   const struct option longopts[] = {
-    {"socket-group",        required_argument,  NULL,   CLI_OPT_SOCKET_GROUP},
-    {"vmnet-mode",          required_argument,  NULL,   CLI_OPT_VMNET_MODE},
-    {"vmnet-interface",     required_argument,  NULL,   CLI_OPT_VMNET_INTERFACE},
-    {"vmnet-gateway",       required_argument,  NULL,   CLI_OPT_VMNET_GATEWAY},
-    {"vmnet-dhcp-end",      required_argument,  NULL,   CLI_OPT_VMNET_DHCP_END},
-    {"vmnet-mask",          required_argument,  NULL,   CLI_OPT_VMNET_MASK},
-    {"vmnet-interface-id",  required_argument,  NULL,   CLI_OPT_VMNET_INTERFACE_ID},
-    {"vmnet-nat66-prefix",  required_argument,  NULL,   CLI_OPT_VMNET_NAT66_PREFIX},
-    {"pidfile",             required_argument,  NULL,   'p'},
-    {"help",                no_argument,        NULL,   'h'},
-    {"version",             no_argument,        NULL,   'v'},
-    {0, 0, 0, 0},
+      {"socket-group",       required_argument, NULL, CLI_OPT_SOCKET_GROUP      },
+      {"vmnet-mode",         required_argument, NULL, CLI_OPT_VMNET_MODE        },
+      {"vmnet-interface",    required_argument, NULL, CLI_OPT_VMNET_INTERFACE   },
Please run clang-format locally and commit the changes.
+      {"vmnet-gateway",      required_argument, NULL, CLI_OPT_VMNET_GATEWAY     },
+      {"vmnet-dhcp-end",     required_argument, NULL, CLI_OPT_VMNET_DHCP_END    },
+      {"vmnet-mask",         required_argument, NULL, CLI_OPT_VMNET_MASK        },

We should log everything to stdout or stderr, we cannot use both. Since logging to stderr is harder and, lets use only stdout.

we should remove >&2 for stdout it will give simple and clear stdout I think.

and for format check for .clang-format

- name: Validate .clang-format with yamllint
        run: |
          echo "Checking .clang-format for YAML syntax..."
          brew install yamllint
          yamllint -d "{extends: relaxed, rules: {line-length: {max: 120}}}" .clang-format

is it okay

nirs · 2025-04-04T19:13:58Z

name: Validate .clang-format with yamllint
run: |
echo "Checking .clang-format for YAML syntax..."
brew install yamllint
yamllint -d "{extends: relaxed, rules: {line-length: {max: 120}}}" .clang-format

I don't think it is needed since we one trivial yaml file, and the code checking it is longer than the actual file.

.github/workflows/build.yaml

nirs · 2025-04-04T20:38:55Z

@saurabhraghuvanshii the CI output is very nice now!

To finish this change we need to:

Squash all the commit into one
- Run git rebase -i master
- Change all lines (expect the first line) to start with "fixup" instead of "pick"
- Save and exit
Fix the commit message
- The title should explain what this change is doing
- The body should explain why
- Have one Signed-off-by line at the end
Add a commit changing the code format to make the CI pass
Reorder the commits
- Run git rebase -i master
- Move the code change commit before the CI job commit

Signed-off-by: saurabhraghuvanshii <saurabhsraghuvanshi@gmail.com>

This change introduces a .clang-format file based on LLVM style with custom rules, and adds a GitHub Actions CI job that checks on build of formatting code. Signed-off-by: saurabhraghuvanshii <saurabhsraghuvanshi@gmail.com>

saurabhraghuvanshii · 2025-04-05T07:30:02Z

@nirs done sir

nirs

Thank you @saurabhraghuvanshii!

Requested changes have been made

saurabhraghuvanshii · 2025-04-06T06:58:41Z

Thanks to all for supports and for improving my knowledge.

jandubois reviewed Feb 24, 2025

View reviewed changes

.github/workflows/clang-format.yaml Outdated Show resolved Hide resolved

nirs reviewed Feb 24, 2025

View reviewed changes

.clang-format Show resolved Hide resolved

.clang-format Outdated Show resolved Hide resolved

.clang-format Outdated Show resolved Hide resolved

.github/workflows/clang-format.yaml Outdated Show resolved Hide resolved

.github/workflows/clang-format.yaml Outdated Show resolved Hide resolved

saurabhraghuvanshii force-pushed the Clang-format branch 2 times, most recently from aa5a225 to e991931 Compare February 25, 2025 15:31

nirs reviewed Feb 26, 2025

View reviewed changes

.github/workflows/build.yaml Outdated Show resolved Hide resolved

.github/workflows/build.yaml Outdated Show resolved Hide resolved

.github/workflows/build.yaml Outdated Show resolved Hide resolved

jandubois requested a review from nirs March 9, 2025 06:36

saurabhraghuvanshii requested a review from jandubois March 26, 2025 15:50

nirs reviewed Apr 2, 2025

View reviewed changes

.clang-format Outdated Show resolved Hide resolved

.clang-format Outdated Show resolved Hide resolved

.clang-format Outdated Show resolved Hide resolved

.github/workflows/build.yaml Outdated Show resolved Hide resolved

saurabhraghuvanshii force-pushed the Clang-format branch 2 times, most recently from 880e817 to dedbe95 Compare April 3, 2025 08:01

saurabhraghuvanshii force-pushed the Clang-format branch 2 times, most recently from 7254a34 to f20adc7 Compare April 3, 2025 12:13

AkihiroSuda reviewed Apr 3, 2025

View reviewed changes

.clang-format Show resolved Hide resolved

AkihiroSuda reviewed Apr 3, 2025

View reviewed changes