public inbox for gwml@vger.gnuweeb.org
 help / color / mirror / Atom feed
From: Alviro Iskandar Setiawan <alviro.iskandar@gnuweeb.org>
To: Ahmad Gani <reyuki@gnuweeb.org>
Cc: Ammar Faizi <ammarfaizi2@gnuweeb.org>,
	"GNU/Weeb Mailing List" <gwml@vger.gnuweeb.org>
Subject: Re: [PATCH gwproxy v2 1/3] dnslookup: split common functionality and struct into net.c
Date: Sat, 2 Aug 2025 06:43:47 +0700	[thread overview]
Message-ID: <CAOG64qN95Voxe5yu8ppb2N=s7Hs2UbE_Yg8Q4ARjod9Cuka0Lw@mail.gmail.com> (raw)
In-Reply-To: <20250801015427.439511-2-reyuki@gnuweeb.org>

On Fri, Aug 1, 2025 at 8:55 AM Ahmad Gani wrote:
> +__cold
> +int convert_str_to_ssaddr(const char *str, struct gwp_sockaddr *gs, uint16_t prt)

(( Nit: Change the var name from prt to port. The naming prt sounds weird. ))

That port parameter addition should not be separate. So this should be
broken down into two commits:

   - Move convert_ssaddr_to_str to net.c.
   - Add a new parameter, default_port.

Also, you need to explain the reason why the port parameter is needed
in the commit message. For now, I don't see any explanation in this
patch why the port parameter is needed.

-- Viro

  reply	other threads:[~2025-08-01 23:43 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-08-01  1:54 [PATCH gwproxy v2 0/3] Initial work for DNS lookup implementation Ahmad Gani
2025-08-01  1:54 ` [PATCH gwproxy v2 1/3] dnslookup: split common functionality and struct into net.c Ahmad Gani
2025-08-01 23:43   ` Alviro Iskandar Setiawan [this message]
2025-08-01 23:45     ` Alviro Iskandar Setiawan
2025-08-02  1:08       ` reyuki
2025-08-02  1:34         ` Alviro Iskandar Setiawan
2025-08-01  1:54 ` [PATCH gwproxy v2 2/3] dnslookup: Allow only port string number Ahmad Gani
2025-08-01  1:54 ` [PATCH gwproxy v2 3/3] dnslookup: Initial work for implementation of C-ares-like getaddrinfo function Ahmad Gani
2025-08-02  1:37   ` Alviro Iskandar Setiawan
2025-08-02  2:06     ` reyuki

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAOG64qN95Voxe5yu8ppb2N=s7Hs2UbE_Yg8Q4ARjod9Cuka0Lw@mail.gmail.com' \
    --to=alviro.iskandar@gnuweeb.org \
    --cc=ammarfaizi2@gnuweeb.org \
    --cc=gwml@vger.gnuweeb.org \
    --cc=reyuki@gnuweeb.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox