Remove unused URL helper functions

[shinrich]

No description provided.

[shinrich]

[approve ci autest]

[maskit]

We might want to call those functions from URL class' getters, instead of having the code in the getters. That's what we do for the setters, although I don't know why we need URL and URLImpl.

[SolidWallOfCode]

I think that should be a separate PR - at this point, if these are not used, it's reasonably to just KIWF.

[maskit]

Remove the functions on this PR, and then reimplement exactly the same functions on next PR to call them from URL class? That doesn't make sense. If you're going that way, I'm going to be -1 on this PR and create "a separate PR" to avoid reimplementing the functions.

This is what you are removing

trafficserver/proxy/hdrs/URL.cc

Lines 674 to 679 in 84cf02e

	const char *
	url_user_get(URLImpl *url, int *length)
	{
	*length = url->m_len_user;
	return url->m_ptr_user;
	}

These are what we have in a different place

trafficserver/proxy/hdrs/URL.h

Lines 542 to 548 in 84cf02e

	inline const char *
	URL::user_get(int *length)
	{
	ink_assert(valid());
	*length = m_url_impl->m_len_user;
	return m_url_impl->m_ptr_user;
	}

trafficserver/proxy/hdrs/URL.h

Lines 553 to 558 in 84cf02e

	inline void
	URL::user_set(const char *value, int length)
	{
	ink_assert(valid());
	url_user_set(m_heap, m_url_impl, value, length, true);
	}

[shinrich]

So I started to pull the url_*_set functions into the URL methods, but those free functions are also used else where.

The url_get functions are not called from anywhere. So I think this PR as it stands is useful. Removing unused functions. Making the URL set methods symmetric is not beneficial since the url_set free functions will exist in any case.

[maskit]

So I started to pull the url_*_set functions into the URL methods, but those free functions are also used else where.

That is not what I'm suggesting. url_*_set functions are ok as they are.

The url_get functions are not called from anywhere. So I think this PR as it stands is useful. Removing unused functions.

I'm suggesting to use those url_*_get functions in URL class' methods.

inline const char * 
URL::user_get(int *length) 
{ 
  ink_assert(valid()); 
  return url_user_get(m_url_impl, length);
} 

No dup code, no unused code, no detail exposure, and no inconsistency. And once we do this, making all the free functions to URLImpl's methods is probably just a couple of regex operations since the first parameter is a pointer for URLImpl instance.

I think this is the right direction, because I'd have to add back the unused functions to achieve no detail exposure and no inconsistency if you remove the functions. It's not a big deal, but I'm not going to approve removing the functions for the reasons above. If you disagree find someone else to get an approval.

[shinrich]

I don't care much one way or the other. Just a tidy up PR anyway.