raindrops RubyGem user+dev discussion/patches/pulls/bugs/help
 help / Atom feed
* Re: [PATCH] Remove Scope IDs from IPv6 addresses.
       [not found] ` <20130910183504.GA502@dcvr.yhbt.net>
@ 2013-09-10 18:36   ` Eric Wong
  2013-09-10 19:57     ` Hleb Valoshka
  0 siblings, 1 reply; 4+ messages in thread
From: Eric Wong @ 2013-09-10 18:36 UTC (permalink / raw)
  To: raindrops

(Moving to the right list, this thread was originally posted to kgio :x)

Hleb Valoshka <375gnu@gmail.com> wrote:
> 	Scoped ipv6 addresses are defined in rfc4007.
> 	Ruby doesn't support them yet and it's unknown whether it will
> 	(see http://bugs.ruby-lang.org/issues/8464).
> 	So we just remove scope ids.

Thanks.  Comments inline.

> --- a/ext/raindrops/linux_inet_diag.c
> +++ b/ext/raindrops/linux_inet_diag.c
> @@ -134,17 +134,54 @@ static int st_free_data(st_data_t key, st_data_t value, st_data_t ignored)

> +static char* remove_scope_id(const char * addr)
> +{
> +        char *newaddr, *t;
> +
> +        newaddr = strdup(addr);
> +        if (newaddr == NULL)
> +                perror("strdup");

We'll segfault on strchr below if we perrored above.  If we continue
using strdup, I would just use ruby_strdup() (works on both MRI/rbx).

But I'd rather avoid *strdup entirely.  This is unlikely to
remove, right?

> +        t = strchr(newaddr, '%');
> +
> +        if (t != NULL) {
> +                *t = '\0';
> +                t = strchr(t+1, ']');
> +                if (t == NULL)
> +                        fprintf(stderr, "Bad IPv6 address: %s!\n", addr);

Better to use rb_warn/rb_warning.
These are preferable since they allow assigning $stderr to StringIO and
such.  But if we don't use strdup, maybe we could raise instead.

> +                else
> +                        strcat(newaddr, t);
> +        }
> +
> +        return newaddr;
> +}

Perhaps something like this (totally untested, likely off-by-one errors)

static VALUE remove_scope_id(const char *addr)
{
	VALUE rv = rb_str_new2(addr);
	long len = RSTRING_LEN(rv);
	char *ptr = RSTRING_PTR(rv);
	char *pct = memchr(ptr, '%', len);

	/*
	 * remove scoped portion
	 * Ruby equivalent: rv.sub!(/%([^\]]*)\]/, "]")
	 */
	if (pct) {
		long newlen = pct - ptr;
		char *rbracket = memchr(pct, ']', len - newlen);

		if (rbracket) {
			size_t move = len - (rbracket - ptr);

			memmove(pct, rbracket, move);
			newlen += move;

			rb_str_set_len(rv, newlen);
		} else {
			/* perhaps raise here? */
			rb_warn("Bad IPv6 address: %s", addr);
		}
	}
	return rv;
}
-- 
Eric Wong


^ permalink raw reply	[flat|threaded] 4+ messages in thread

* Re: Re: [PATCH] Remove Scope IDs from IPv6 addresses.
  2013-09-10 18:36   ` [PATCH] Remove Scope IDs from IPv6 addresses Eric Wong
@ 2013-09-10 19:57     ` Hleb Valoshka
  2013-09-10 20:17       ` Eric Wong
  0 siblings, 1 reply; 4+ messages in thread
From: Hleb Valoshka @ 2013-09-10 19:57 UTC (permalink / raw)
  To: raindrops

On 9/10/13, Eric Wong <normalperson@yhbt.net> wrote:
>> +        newaddr = strdup(addr);
>> +        if (newaddr == NULL)
>> +                perror("strdup");
>
> We'll segfault on strchr below if we perrored above.  If we continue
> using strdup, I would just use ruby_strdup() (works on both MRI/rbx).

I haven't use C for a long time, so I thought that strchr checks its args.

> But I'd rather avoid *strdup entirely.  This is unlikely to
> remove, right?

Frankly speaking I dislike functions similar to strdup(), because
using them you should remember to call free(), sometimes in many
places (or to use goto). But I'm unfamiliar with internal ruby api, so
it was the only variant for me.

BTW, is anything bad with that variant:
char *newaddr = alloca(...);
remove_scope_id(oldaddr, newaddr);

> Perhaps something like this (totally untested, likely off-by-one errors)

Ok, I'll test your code tomorrow.


^ permalink raw reply	[flat|threaded] 4+ messages in thread

* Re: Re: [PATCH] Remove Scope IDs from IPv6 addresses.
  2013-09-10 19:57     ` Hleb Valoshka
@ 2013-09-10 20:17       ` Eric Wong
  2013-09-11 16:12         ` Hleb Valoshka
  0 siblings, 1 reply; 4+ messages in thread
From: Eric Wong @ 2013-09-10 20:17 UTC (permalink / raw)
  To: raindrops

Hleb Valoshka <375gnu@gmail.com> wrote:
> On 9/10/13, Eric Wong <normalperson@yhbt.net> wrote:
> >> +        newaddr = strdup(addr);
> >> +        if (newaddr == NULL)
> >> +                perror("strdup");
> >
> > We'll segfault on strchr below if we perrored above.  If we continue
> > using strdup, I would just use ruby_strdup() (works on both MRI/rbx).
> 
> I haven't use C for a long time, so I thought that strchr checks its args.
> 
> > But I'd rather avoid *strdup entirely.  This is unlikely to
> > remove, right?
> 
> Frankly speaking I dislike functions similar to strdup(), because
> using them you should remember to call free(), sometimes in many
> places (or to use goto). But I'm unfamiliar with internal ruby api, so
> it was the only variant for me.

I try to avoid strdup, too.  I can help you with Ruby C API :)
The advantage of creating Ruby strings right away is it maintains
length, so one can use mem* functions instead of str* functions
(always faster and more explicit, often safer, and never less safe).

> BTW, is anything bad with that variant:
> char *newaddr = alloca(...);
> remove_scope_id(oldaddr, newaddr);

Probably OK in this case, but it would need a comment explaining to stack
checkers the size is bounded and won't overflow.

> > Perhaps something like this (totally untested, likely off-by-one errors)
> 
> Ok, I'll test your code tomorrow.

Thanks.


^ permalink raw reply	[flat|threaded] 4+ messages in thread

* Re: Re: [PATCH] Remove Scope IDs from IPv6 addresses.
  2013-09-10 20:17       ` Eric Wong
@ 2013-09-11 16:12         ` Hleb Valoshka
  0 siblings, 0 replies; 4+ messages in thread
From: Hleb Valoshka @ 2013-09-11 16:12 UTC (permalink / raw)
  To: raindrops

On 9/10/13, Eric Wong <normalperson@yhbt.net> wrote:
>> > Perhaps something like this (totally untested, likely off-by-one
>> > errors)
>> Ok, I'll test your code tomorrow.
> Thanks.

Okay, it works, but I was too busy today to test it enough. I hope tomorrow.


^ permalink raw reply	[flat|threaded] 4+ messages in thread

end of thread, back to index

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <1378826261-10771-1-git-send-email-375gnu@gmail.com>
     [not found] ` <20130910183504.GA502@dcvr.yhbt.net>
2013-09-10 18:36   ` [PATCH] Remove Scope IDs from IPv6 addresses Eric Wong
2013-09-10 19:57     ` Hleb Valoshka
2013-09-10 20:17       ` Eric Wong
2013-09-11 16:12         ` Hleb Valoshka

raindrops RubyGem user+dev discussion/patches/pulls/bugs/help

Archives are clonable:
	git clone --mirror https://bogomips.org/raindrops-public
	git clone --mirror http://ou63pmih66umazou.onion/raindrops-public

Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.lang.ruby.raindrops
	nntp://ou63pmih66umazou.onion/inbox.comp.lang.ruby.raindrops

 note: .onion URLs require Tor: https://www.torproject.org/
       or Tor2web: https://www.tor2web.org/

AGPL code for this site: git clone https://public-inbox.org/ public-inbox