Code comments

[hao-tian-xu]

Related to #368
Hi, I formatted my code comments of the main function to see if the approach is ok, like correctness, verbose, etc.

Other code comments are here: hao-tian-xu:comments. I will format and pr them afterwards.

[jesperpedersen]

I think this is a bit too detailed documented.

The documentation should give the reader a sense of what is going on in a code block, like

   /* Process command line arguments */
   while (1)
   {
      static struct option long_options[] =
      {
         {"config", required_argument, 0, 'c'},
         {"hba", required_argument, 0, 'a'},
         {"limit", required_argument, 0, 'l'},
         {"users", required_argument, 0, 'u'},
         {"frontend", required_argument, 0, 'F'},
         {"admins", required_argument, 0, 'A'},
         {"superuser", required_argument, 0, 'S'},
         {"daemon", no_argument, 0, 'd'},
         {"version", no_argument, 0, 'V'},
         {"help", no_argument, 0, '?'}
      };
      int option_index = 0;

      c = getopt_long (argc, argv, "dV?a:c:l:u:F:A:S:",
                       long_options, &option_index);

      // Break the loop if there are no more options to process
      if (c == -1)
      {
         break;
      }

      // Process the current option
      switch (c)
      {
         case 'a':
            hba_path = optarg;
            break;
         case 'c':
            configuration_path = optarg;
            break;
         case 'l':
            limit_path = optarg;
            break;
         case 'u':
            users_path = optarg;
            break;
         case 'F':
            frontend_users_path = optarg;
            break;
         case 'A':
            admins_path = optarg;
            break;
         case 'S':
            superuser_path = optarg;
            break;
         case 'd':
            daemon = true;
            break;
         case 'V':
            version();
            break;
         case '?':
            usage();
            exit(1);
            break;
         default:
            break;
      }
   }

We should assume that people know most of the standard functions, so we don't have to explain what memset and so on does. What is important is to explain that the struct configuration is initialized, like

  /* Initialize the main configuration structure, and load the configuration from the specified file or default location */

or something like that. Similar for the other files - less line-by-line description but a bigger block of what/why/how/... in the beginning.

I don't think we need to document the actual variables - the documentation before the code block where they are should be enough to get the picture, so

   /* Run the main loop */
   while (keep_running)
   {
      ev_loop(main_loop, 0);
   }

should be enough.

Of course, there is always a balance... Ideally, documentation for the function should be enough to get an idea, but only the .h have brief documentation. The .c could have a deeper description maybe even with some design choices...

Give it another spin and lets look at it again.

Side note, I prefer the /* */ syntax

[hao-tian-xu]

Thanks for the feedback! I gave it another try to find a balance between concise, informative, and complete.

Any further feedback for improvement would be great, since I'd gradually format other code comments similarly. Hope this work will help future newcomers :)

[jesperpedersen]

Yeah, stuff like this

[jesperpedersen]

The struct documentation could be expanded to explain what it holds, and is used for.

The function documentation could give a deeper explanation, and inside .c even design decisions

[hao-tian-xu]

Thanks for the further feedback! Just came back from holiday. Will continually work on this while exploring and working on other issues.

[jesperpedersen]

Sounds good - once you have something that you are happy with as the first update take the pull request out of draft, and send a PTAL

[jesperpedersen]

Please, reopen if you think it is necessary with an up-to-date version